> > >> + >> +/* >> + * this journal write must contain full parity, >> + * it may also contain some data pages >> + */ >> +static void r5c_handle_parity_cached(struct stripe_head *sh) > > I don't understand how this function name corresponds to what it does or > when it is called. > It is parts of activating the WRITE_OUT action for a stripe that has (or > may have) been cached to the journal. None of that is particularly > about "parity". It is called parity cached, because it is part of endio of the parity write (to journal). We only write RAID disks (write out) after all parities are in journal. So if we name the function for its behavior, it will be something like "start_write_out_to_raid" > > In generally the patch looks good, but it bothers me that we need to add > tests on R5_InJournal in lots and lots of places. It makes all those > test sites more complex and so easily misunderstood. > I wonder if there is some way we could add a new flag or something that > would subsumes several of the tests. So instead of adding a test for InJournal, > we could replace the current test with a test for something new? > Or maybe gather several tests that appear together into an inline. > Or something. This is a great point. Let me try that. Thanks, Song -- To unsubscribe from this list: send the line "unsubscribe linux-raid" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
