No problem. I will change the patch to remove this extra read. Thanks. On Mon, Dec 9, 2019 at 5:52 PM Coly Li <colyli@xxxxxxx> wrote: > > On 2019/12/9 3:37 下午, Christoph Hellwig wrote: > > On Fri, Dec 06, 2019 at 05:44:38PM +0800, Coly Li wrote: > >>> { > >>> - struct cache_sb *out = page_address(bio_first_page_all(bio)); > >>> + struct cache_sb *out; > >>> unsigned int i; > >>> + struct buffer_head *bh; > >>> + > >>> + /* > >>> + * The page is held since read_super, this __bread * should not > >>> + * cause an extra io read. > >>> + */ > >>> + bh = __bread(bdev, 1, SB_SIZE); > >>> + if (!bh) > >>> + goto out_bh; > >>> + > >>> + out = (struct cache_sb *) bh->b_data; > >> > >> This is quite tricky here. Could you please to move this code piece into > >> an inline function and add code comments to explain why a read is > >> necessary for a write. > > > > A read is not nessecary. He only added it because he was too fearful > > of calculating the data offset directly. But calculating it directly > > is almost trivial and should just be done here. Alternatively if that > > is still to hard just keep a pointer to the cache_sb around, which is > > how most file systems do it. > > > Copied, if Liang does not have time to handle this as your suggestion, I > will handle it. > > Thanks for the hint. > > -- > > Coly Li