Re: [PATCH] btrfs: update device path inode time instead of bd_inode

[Josef Bacik <josef@toxicpanda.com>]

On Thu, Oct 14, 2021 at 05:53:41PM +0200, Christoph Hellwig wrote:
> On Thu, Oct 14, 2021 at 11:50:07AM -0400, Josef Bacik wrote:
> > On Thu, Oct 14, 2021 at 05:33:47PM +0200, Christoph Hellwig wrote:
> > > On Thu, Oct 14, 2021 at 11:17:08AM -0400, Josef Bacik wrote:
> > > > +	now = current_time(d_inode(path.dentry));
> > > > +	generic_update_time(d_inode(path.dentry), &now, S_MTIME | S_CTIME);
> > > 
> > > This is still broken as it won't call into ->update_time.
> > > generic_update_time is a helper/default for ->update_time, not something
> > > for an external caller.
> > 
> > Then we probably need to fix all the people currently calling it.
> 
> All other callers are from update_time / ->update_time.
> 

I'm seeing a lot more calls to generic_update_time elsewhere

  File                Function             Line
0 fs/inode.c          <global>             1779 EXPORT_SYMBOL(generic_update_time);
1 fs/btrfs/volumes.c  update_dev_time      1900 generic_update_time(d_inode(path.dentry), &now, S_MTIME | S_CTIME);
2 fs/gfs2/inode.c     gfs2_update_time     2146 return generic_update_time(inode, time, flags);
3 fs/inode.c          generic_update_time  1755 int generic_update_time(struct inode *inode, struct timespec64 *time, int flags)
4 fs/inode.c          inode_update_time    1789 return generic_update_time(inode, time, flags);
5 fs/orangefs/inode.c orangefs_update_time  913 generic_update_time(inode, time, flags);
6 fs/ubifs/file.c     ubifs_update_time    1382 return generic_update_time(inode, time, flags);
7 fs/xfs/xfs_iops.c   xfs_vn_update_time   1123 return generic_update_time(inode, now, flags);
8 include/linux/fs.h  __printf             2617 extern int generic_update_time(struct inode *, struct timespec64 *, int );

> > In the
> > meantime I'll add a helper to use the thing that calls ->update_time instead.
> > Thanks,
> 
> Looking at this a bit more I think the right fix is to simply revert the
> offending commit.  The lockdep complains was due to changes issues in the
> loop driver and has been fixed in the loop driver in the meantime.
> 

Where were they fixed?  And it doesn't fix the fact that we're calling open on a
device, so any change at all to the loop device is going to end us back up in
this spot because we end up with the ->lo_mutex in our dependency chain.  I want
to avoid this by not calling open, and that means looking up the inode and doing
operations without needing to go through the full file open path.

The best thing for btrfs here is to export the update_time() helper and call
that to avoid all the baggage that comes from opening the block device.  Thanks,

Josef