Ted-san, Theodore Tso wrote: > On Fri, Dec 12, 2008 at 09:54:18AM +0900, Toshiyuki Okajima wrote: > > > To tell the truth, at first, I imagined the same patch as yours to fix this > > > problem. But I have made another patch because I thought that ext3(or ext4) > > > should not know the contents of the processing of journal_try_to_free_buffers > > > in detail. (ext3 should not know there is a possibility to call > > > journal_wait_for_transaction_sync_data from journal_try_to_free_buffers.) > > I agree, but ext3 doesn't need to know that. What my change did was > to mask off the _GFP_WAIT flag, which prohibits the function it calls > from blocking, because it knows that its caller is holding a spinlock. > > And actually, come to think of it. We can do even better; the right > fix is to have blkdev_releasepage() mask off the _GFP_WAIT flag; this > is the function which is taking spinlock, and by masking off the > __GFP_WAIT flag, this is simply requesting all of the downstream > functions not to block, but to do the best job they can do without > blocking. It doesn't need to know whether it's going to call > log_wait_commit(), or anything else; all it needs to do is request > "please don't block". > That means we only make the request once, in the function which is > taking spinlock, so all of the per-filesystem implementations of > release_metadata() don't need to know that its caller is holding a > spinlock. OK. I agree your last change. I also think blkdev_releasepage must do something so that downstream functions of it don't sleep. Masking off the _GFP_WAIT flag is the easiest achievement of it. Besides, I think it is not valid implementation that brings us a care about ei->client_releasepage's sleeping. Additional Information: I did an easy test with your last change and then I haven't experienced any errors. Regards, Toshiyuki Okajima -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
