Project

General

Profile

Bug #4777

krbd: verify a few things in the zeroing routines

Added by Alex Elder almost 9 years ago. Updated over 8 years ago.

Status:
Resolved
Priority:
High
Assignee:
Target version:
-
% Done:

0%

Source:
Development
Tags:
Backport:
Regression:
No
Severity:
3 - minor
Reviewed:
Affected Versions:
ceph-qa-suite:
Pull request ID:
Crash signature (v1):
Crash signature (v2):

Description

The kernel rbd driver has a function zero_bio_chain() that's
used to zero out the data in a bio list starting at a given
offset and extending to the end of the bio vectors in the list.

I've now added a similar function, zero_pages(), which does
the same sort of thing across a range of pages in a page array.
In order to mimic what zero_bio_chain() does I saved and
restored the irq flags around zeroing the data.

I don't think that's needed, and Josh didn't either. But
since I don't really know why it's there in the original
function I'm leaving it in until I can take a closer look.

So: the first thing is I want to get rid of the save/restore
irq flags in zero_pages(), and in zero_bio_chain() as well
if that's possible.

Second, the kernel has its own version of the bio function,
zero_fill_bio(). That includes a call to flush_dcache_page()
after zeroing the data in a bio vector. I think we need the
same thing in our zeroing routines. For x86 this is a no-op,
but for portability to a few other architectures (including ARM)
if it's necessary at all, we need it.

History

#1 Updated by Alex Elder over 8 years ago

  • Priority changed from Normal to High

#2 Updated by Alex Elder over 8 years ago

I'm going to ignore the whole IRC issue for now.
I'm almost certain it's not needed, and in fact
I'm pretty sure it's not even needed in the
kernel's zero_fill_bio() routine. But that can
be addressed separately. I'm planning to leave
in the irq save/restore flags in both zero_pages()
and zero_bio_chain().

The other issue though is important. Some architectures
(like ARM) have caches that allow more than one virtual
address to map to the same physical (or virtual) address.
This means that whenever the kernel updates memory that
might be aliased in this way, it needs to flush out any
caches that might contain old copies of the data that
was found in the updated location.

#3 Updated by Alex Elder over 8 years ago

  • Status changed from New to Fix Under Review

The following patch has been posted for review. It's one of three
new patches available in the "review/wip-rbd" branch of the
ceph-client git repository.

[PATCH] rbd: flush dcache after zeroing page data

#4 Updated by Alex Elder over 8 years ago

  • Status changed from Fix Under Review to Resolved

The following has been committed to the "testing" branch
of the ceph-client git respository:

81d7ac5 rbd: flush dcache after zeroing page data

Also available in: Atom PDF