Ted-san, Thank you for your reviewing. > Toshiyuki-san, > > My apologies for not having a chance to review your patches; things > have been rather busy for me. There were a couple of shortcomings in > your patch series, and I think there is a better way of solving the > issue at hand. First of all, patches #1 and #2 use a new function > which is not actually defined until patches #3 and #4, respectively. > This can make it difficult for people who are trying to use "git > bisect" to try to localize the root cause of a problem. > > Secondly, the introduction of a large number of wrapper functions > increases stack utilization, and makes the call depth deeper (and in > the long run, each increase in call depth makes the code that much > harder to trace through and understand), and so it should be done only > as last resort. Fortunately, there is a simpler way of fixing this > problem. I include the inter-diff below, but I will fold this into > the current blkdev_releasepage() patches that are in the ext4 patch > queue. > > Best regards, > > - Ted > > P.S. Note that this patch is functionally identical to what you > proposed in your patch series, but since the gfp_wait mask already > controlls whether or not log_wait_commit() is called, instead of > introducing a new functional parameter, we just mask off __GFP_WAIT > before calling jbd2_journal_try_to_free_buffers. I'll make a similar > change to the ext3 patch, and attach the two revised patches to this > mail thread. Through the idea as follows, I agree to your change. ----------------------------------------------------------------------------- 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.) So, I have made a new function, journal_try_to_free_metadata_buffers to release only metadata buffer_heads. (I wanted a function which is the almost same as journal_try_to_free_buffers except calling journal_wait_for_transaction_sync_data from it.) However, this new function needed big changes, you know. I reconsidered what was the most suitable patch to fix this problem after I read your mail (patch). And then, I thought that it was important to make the most concise patch to fix only a root cause. Big patch is not easy to understand even if it is more logical one. Therefore, there is the fact that ext3_release_metadata must not sleep because it can get a spinlock, and then, only changing ext3_release_metadata to the logic to make it not sleep is the simplest fix for this problem. ----------------------------------------------------------------------------- Best 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
