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.