All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vivek Goyal <vgoyal@redhat.com>
To: Amir Goldstein <amir73il@gmail.com>
Cc: overlayfs <linux-unionfs@vger.kernel.org>,
	Miklos Szeredi <miklos@szeredi.hu>,
	Giuseppe Scrivano <gscrivan@redhat.com>,
	Daniel J Walsh <dwalsh@redhat.com>,
	Steven Whitehouse <swhiteho@redhat.com>,
	Eric Sandeen <sandeen@redhat.com>,
	pmatilai@redhat.com
Subject: Re: [RFC PATCH v2] overlayfs: Provide mount options sync=off/fs to skip sync
Date: Wed, 1 Jul 2020 16:00:12 -0400	[thread overview]
Message-ID: <20200701200012.GE369085@redhat.com> (raw)
In-Reply-To: <CAOQ4uxisFOkQF8eq5ysZYtdfd_Z26r5MHsdr+ozwz4ry+WuUEA@mail.gmail.com>

On Wed, Jul 01, 2020 at 10:08:28PM +0300, Amir Goldstein wrote:
[..]
> > diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
> > index 6918b98faeb6..970319ca1623 100644
> > --- a/fs/overlayfs/readdir.c
> > +++ b/fs/overlayfs/readdir.c
> > @@ -858,11 +858,15 @@ static int ovl_dir_fsync(struct file *file, loff_t start, loff_t end,
> >         struct ovl_dir_file *od = file->private_data;
> >         struct dentry *dentry = file->f_path.dentry;
> >         struct file *realfile = od->realfile;
> > +       struct ovl_fs *ofs = OVL_FS(dentry->d_sb);
> >
> >         /* Nothing to sync for lower */
> >         if (!OVL_TYPE_UPPER(ovl_path_type(dentry)))
> >                 return 0;
> >
> > +       if (ofs->config.nosync || ofs->config.syncfs)
> > +               return 0;
> > +
> 
> Generally looks good, but those test conditions are quite weird IMO.
> I would go for either enum or flags, but not up to me to decide.
> But at the very least use inline helpers ovl_should_fsync(),
> ovl_should_syncfs(), because if we want to add new sync modes or
> whatever going over all those conditions and
> changing them would be sub-optimal.

Ok, I will add inline helpers.

I am not sure, what will I gain by switching to using enums or flags.
It will be just different style to keep same information.

Thanks
Vivek


      reply	other threads:[~2020-07-01 20:00 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-01 17:57 [RFC PATCH v2] overlayfs: Provide mount options sync=off/fs to skip sync Vivek Goyal
2020-07-01 19:08 ` Amir Goldstein
2020-07-01 20:00   ` Vivek Goyal [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200701200012.GE369085@redhat.com \
    --to=vgoyal@redhat.com \
    --cc=amir73il@gmail.com \
    --cc=dwalsh@redhat.com \
    --cc=gscrivan@redhat.com \
    --cc=linux-unionfs@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=pmatilai@redhat.com \
    --cc=sandeen@redhat.com \
    --cc=swhiteho@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.