Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 24 Mar 2023 05:56:10 +0000
From:   Alok Tiagi <aloktiagi@gmail.com>
To:     Matthew Wilcox <willy@infradead.org>
Cc:     viro@zeniv.linux.org.uk, brauner@kernel.org,
        David.Laight@aculab.com, linux-fsdevel@vger.kernel.org,
        linux-kernel@vger.kernel.org, keescook@chromium.org,
        hch@infradead.org, Tycho Andersen <tycho@tycho.pizza>,
        aloktiagi@gmail.com
Subject: Re: [RFC v3 3/3] file, epoll: Implement do_replace() and
 eventpoll_replace()
Message-ID: <ZB07etlmm6torSkz@ip-172-31-38-16.us-west-2.compute.internal>
References: <20230324051526.963702-1-aloktiagi@gmail.com>
 <20230324051526.963702-3-aloktiagi@gmail.com>
 <ZB00Cvib+jyqCR5C@casper.infradead.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZB00Cvib+jyqCR5C@casper.infradead.org>
Precedence: bulk

On Fri, Mar 24, 2023 at 05:24:26AM +0000, Matthew Wilcox wrote:
> On Fri, Mar 24, 2023 at 05:15:26AM +0000, aloktiagi wrote:
> > diff --git a/include/linux/eventpoll.h b/include/linux/eventpoll.h
> > index 3337745d81bd..38904fce3840 100644
> > --- a/include/linux/eventpoll.h
> > +++ b/include/linux/eventpoll.h
> > @@ -25,6 +25,8 @@ struct file *get_epoll_tfile_raw_ptr(struct file *file, int tfd, unsigned long t
> >  /* Used to release the epoll bits inside the "struct file" */
> >  void eventpoll_release_file(struct file *file);
> >  
> > +void eventpoll_replace_file(struct file *toreplace, struct file *file);
> > +
> >  /*
> >   * This is called from inside fs/file_table.c:__fput() to unlink files
> >   * from the eventpoll interface. We need to have this facility to cleanup
> > @@ -53,6 +55,22 @@ static inline void eventpoll_release(struct file *file)
> >  	eventpoll_release_file(file);
> >  }
> >  
> > +
> > +/*
> > + * This is called from fs/file.c:do_replace() to replace a linked file in the
> > + * epoll interface with a new file received from another process. This is useful
> > + * in cases where a process is trying to install a new file for an existing one
> > + * that is linked in the epoll interface
> > + */
> > +static inline void eventpoll_replace(struct file *toreplace, struct file *file)
> > +{
> > +	/*
> > +	 * toreplace is the file being replaced. Install the new file for the
> > +	 * existing one that is linked in the epoll interface
> > +	 */
> > +	eventpoll_replace_file(toreplace, file);
> > +}
> 
> Why do we have both eventpoll_replace() and eventpoll_replace_file()?
> They seem identical?

I was following the eventpoll_release_file() convention but yes seems
unnecessary. I'll retain eventpoll_replace_file() and remove
eventpoll_replace() in v4.

> 
> > diff --git a/include/linux/file.h b/include/linux/file.h
> > index 39704eae83e2..80e56b2b44fb 100644
> > --- a/include/linux/file.h
> > +++ b/include/linux/file.h
> > @@ -36,6 +36,7 @@ struct fd {
> >  	struct file *file;
> >  	unsigned int flags;
> >  };
> > +
> >  #define FDPUT_FPUT       1
> >  #define FDPUT_POS_UNLOCK 2
> >  
> 
> You should drop this hunk of the patch.

I overlooked this. Will fix it in v4.