Re: [PATCH 4/5] ovl: prepare for lazy lookup of lowerdata inode

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Apr 27, 2023 at 2:12 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote:
>
> On Wed, Apr 26, 2023 at 5:57 PM Miklos Szeredi <miklos@xxxxxxxxxx> wrote:
> >
> > On Wed, 12 Apr 2023 at 15:54, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
> > >
> > > Make the code handle the case of numlower > 1 and missing lowerdata
> > > dentry gracefully.
> > >
> > > Missing lowerdata dentry is an indication for lazy lookup of lowerdata
> > > and in that case the lowerdata_redirect path is stored in ovl_inode.
> > >
> > > Following commits will defer lookup and perform the lazy lookup on
> > > acccess.
> > >
> > > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> > > ---
> > >  fs/overlayfs/export.c |  2 +-
> > >  fs/overlayfs/file.c   |  7 +++++++
> > >  fs/overlayfs/inode.c  | 18 ++++++++++++++----
> > >  fs/overlayfs/super.c  |  3 +++
> > >  fs/overlayfs/util.c   |  2 +-
> > >  5 files changed, 26 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/fs/overlayfs/export.c b/fs/overlayfs/export.c
> > > index 9951c504fb8d..2498fa8311e3 100644
> > > --- a/fs/overlayfs/export.c
> > > +++ b/fs/overlayfs/export.c
> > > @@ -343,7 +343,7 @@ static struct dentry *ovl_dentry_real_at(struct dentry *dentry, int idx)
> > >         if (!idx)
> > >                 return ovl_dentry_upper(dentry);
> > >
> > > -       for (i = 0; i < ovl_numlower(oe); i++) {
> > > +       for (i = 0; i < ovl_numlower(oe) && lowerstack[i].layer; i++) {
> >
> > Metacopy and NFS export are mutually exclusive, so this doesn't make sense.
> >
>
> OK.
>
> >
> > > diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> > > index 3484f39a8f27..ef78abc21998 100644
> > > --- a/fs/overlayfs/super.c
> > > +++ b/fs/overlayfs/super.c
> >
> > ovl_d_real() calls ovl_dentry_lowerdata().  If triggered from
> > file_dentry() it should be okay, since that is done on an open file
> > (lazy lookup already perfromed).   But it can also be called from
> > d_real_inode(), the only caller of which is trace_uprobe.  Is this
> > going to be okay?
> >
>
> Not sure.
> It's hard to imagine that trace_uprobe_create() is being called to place
> a probe on a file at offset X without reading the content of symbols first.
>
> I wonder if lazy lookup from within ovl_d_real(d, NULL) is acceptable?
> It does look like the context of trace_uprobe() callers should be fine for lazy
> lowerdata lookup.
>
> > In any case a comment is needed at least.
> >
>
> I will leave it as is and add a comment.

On second thought, I will add best effort lazy lowerdata lookup
in addition to the warning.
d_real_inode() does not expect errors or NULL and trace_uprobe
does not check for them either.

Thanks,
Amir.




[Index of Archives]     [Linux Filesystems Devel]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux