Re: [PATCH v3] fs: Fix page cache inconsistency when mixing buffered and AIO DIO

[Lukas Czerner <lczerner@redhat.com>]

On Tue, Jul 18, 2017 at 06:44:20AM -0700, Christoph Hellwig wrote:
> ->inode->i_sb);
> 
> So now we initialize the workqueue on the first aio write.  Maybe we
> should just always initialize it?  Especially given that the cost of
> a workqueue is rather cheap.  I also don't really understand why
> we even need the workqueue per-superblock instead of global.

As Jan mentioned and I agree it's worth initializing it only when
actually needed.

> 
> > index 1732228..2f8dbf9 100644
> > --- a/fs/iomap.c
> > +++ b/fs/iomap.c
> > @@ -713,8 +713,16 @@ struct iomap_dio {
> >  static ssize_t iomap_dio_complete(struct iomap_dio *dio)
> >  {
> >  	struct kiocb *iocb = dio->iocb;
> > +	loff_t offset = iocb->ki_pos;
> 
> If you introduce this variable please also use it later in the function
> instead of iocb->ki_pos.  OR remove the variable, which would be fine
> with me as well.

Right, I did not use it later in the fucntion because it would be
confusing (we're changing iocb->ki_pos). So I'll just remove the
variable.

> 
> > +	struct inode *inode = file_inode(iocb->ki_filp);
> >  	ssize_t ret;
> >  
> > +	if ((!dio->error) &&
> 
> no need for the inner braces.

ok


Thanks!
-Lukas