| Message ID | 20220426174335.4004987-12-shr@fb.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | io-uring/xfs: support async buffered writes | expand |
On Tue, Apr 26, 2022 at 10:43:28AM -0700, Stefan Roesch wrote: > This adds the async buffered write support to XFS. For async buffered > write requests, the request will return -EAGAIN if the ilock cannot be > obtained immediately. > > Signed-off-by: Stefan Roesch <shr@fb.com> > --- > fs/xfs/xfs_file.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c > index 6f9da1059e8b..49d54b939502 100644 > --- a/fs/xfs/xfs_file.c > +++ b/fs/xfs/xfs_file.c > @@ -739,12 +739,14 @@ xfs_file_buffered_write( > bool cleared_space = false; > int iolock; > > - if (iocb->ki_flags & IOCB_NOWAIT) > - return -EOPNOTSUPP; > - > write_retry: > iolock = XFS_IOLOCK_EXCL; > - xfs_ilock(ip, iolock); > + if (iocb->ki_flags & IOCB_NOWAIT) { > + if (!xfs_ilock_nowait(ip, iolock)) > + return -EAGAIN; > + } else { > + xfs_ilock(ip, iolock); > + } xfs_ilock_iocb(). -Dave.
On 4/26/22 3:56 PM, Dave Chinner wrote: > On Tue, Apr 26, 2022 at 10:43:28AM -0700, Stefan Roesch wrote: >> This adds the async buffered write support to XFS. For async buffered >> write requests, the request will return -EAGAIN if the ilock cannot be >> obtained immediately. >> >> Signed-off-by: Stefan Roesch <shr@fb.com> >> --- >> fs/xfs/xfs_file.c | 10 ++++++---- >> 1 file changed, 6 insertions(+), 4 deletions(-) >> >> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c >> index 6f9da1059e8b..49d54b939502 100644 >> --- a/fs/xfs/xfs_file.c >> +++ b/fs/xfs/xfs_file.c >> @@ -739,12 +739,14 @@ xfs_file_buffered_write( >> bool cleared_space = false; >> int iolock; >> >> - if (iocb->ki_flags & IOCB_NOWAIT) >> - return -EOPNOTSUPP; >> - >> write_retry: >> iolock = XFS_IOLOCK_EXCL; >> - xfs_ilock(ip, iolock); >> + if (iocb->ki_flags & IOCB_NOWAIT) { >> + if (!xfs_ilock_nowait(ip, iolock)) >> + return -EAGAIN; >> + } else { >> + xfs_ilock(ip, iolock); >> + } > > xfs_ilock_iocb(). > The helper xfs_ilock_iocb cannot be used as it hardcoded to use iocb->ki_filp to get a pointer to the xfs_inode. However here we need to use iocb->ki_filp->f_mapping->host. I'll split off new helper for this in the next version of the patch. > -Dave. >
On Thu, Apr 28, 2022 at 12:58:59PM -0700, Stefan Roesch wrote: > > > On 4/26/22 3:56 PM, Dave Chinner wrote: > > On Tue, Apr 26, 2022 at 10:43:28AM -0700, Stefan Roesch wrote: > >> This adds the async buffered write support to XFS. For async buffered > >> write requests, the request will return -EAGAIN if the ilock cannot be > >> obtained immediately. > >> > >> Signed-off-by: Stefan Roesch <shr@fb.com> > >> --- > >> fs/xfs/xfs_file.c | 10 ++++++---- > >> 1 file changed, 6 insertions(+), 4 deletions(-) > >> > >> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c > >> index 6f9da1059e8b..49d54b939502 100644 > >> --- a/fs/xfs/xfs_file.c > >> +++ b/fs/xfs/xfs_file.c > >> @@ -739,12 +739,14 @@ xfs_file_buffered_write( > >> bool cleared_space = false; > >> int iolock; > >> > >> - if (iocb->ki_flags & IOCB_NOWAIT) > >> - return -EOPNOTSUPP; > >> - > >> write_retry: > >> iolock = XFS_IOLOCK_EXCL; > >> - xfs_ilock(ip, iolock); > >> + if (iocb->ki_flags & IOCB_NOWAIT) { > >> + if (!xfs_ilock_nowait(ip, iolock)) > >> + return -EAGAIN; > >> + } else { > >> + xfs_ilock(ip, iolock); > >> + } > > > > xfs_ilock_iocb(). > > > > The helper xfs_ilock_iocb cannot be used as it hardcoded to use iocb->ki_filp to > get a pointer to the xfs_inode. And the problem with that is? I mean, look at what xfs_file_buffered_write() does to get the xfs_inode 10 lines about that change: xfs_file_buffered_write( struct kiocb *iocb, struct iov_iter *from) { struct file *file = iocb->ki_filp; struct address_space *mapping = file->f_mapping; struct inode *inode = mapping->host; struct xfs_inode *ip = XFS_I(inode); In what cases does file_inode(iocb->ki_filp) point to a different inode than iocb->ki_filp->f_mapping->host? The dio write path assumes that file_inode(iocb->ki_filp) is correct, as do both the buffered and dio read paths. What makes the buffered write path special in that file_inode(iocb->ki_filp) is not correctly set whilst iocb->ki_filp->f_mapping->host is? Regardless, if this is a problem, then just pass the XFS inode to xfs_ilock_iocb() and this is a moot point. > However here we need to use iocb->ki_filp->f_mapping->host. > I'll split off new helper for this in the next version of the patch. We don't need a new helper here, either. Cheers, Dave.
On 4/28/22 2:54 PM, Dave Chinner wrote: > On Thu, Apr 28, 2022 at 12:58:59PM -0700, Stefan Roesch wrote: >> >> >> On 4/26/22 3:56 PM, Dave Chinner wrote: >>> On Tue, Apr 26, 2022 at 10:43:28AM -0700, Stefan Roesch wrote: >>>> This adds the async buffered write support to XFS. For async buffered >>>> write requests, the request will return -EAGAIN if the ilock cannot be >>>> obtained immediately. >>>> >>>> Signed-off-by: Stefan Roesch <shr@fb.com> >>>> --- >>>> fs/xfs/xfs_file.c | 10 ++++++---- >>>> 1 file changed, 6 insertions(+), 4 deletions(-) >>>> >>>> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c >>>> index 6f9da1059e8b..49d54b939502 100644 >>>> --- a/fs/xfs/xfs_file.c >>>> +++ b/fs/xfs/xfs_file.c >>>> @@ -739,12 +739,14 @@ xfs_file_buffered_write( >>>> bool cleared_space = false; >>>> int iolock; >>>> >>>> - if (iocb->ki_flags & IOCB_NOWAIT) >>>> - return -EOPNOTSUPP; >>>> - >>>> write_retry: >>>> iolock = XFS_IOLOCK_EXCL; >>>> - xfs_ilock(ip, iolock); >>>> + if (iocb->ki_flags & IOCB_NOWAIT) { >>>> + if (!xfs_ilock_nowait(ip, iolock)) >>>> + return -EAGAIN; >>>> + } else { >>>> + xfs_ilock(ip, iolock); >>>> + } >>> >>> xfs_ilock_iocb(). >>> >> >> The helper xfs_ilock_iocb cannot be used as it hardcoded to use iocb->ki_filp to >> get a pointer to the xfs_inode. > > And the problem with that is? > > I mean, look at what xfs_file_buffered_write() does to get the > xfs_inode 10 lines about that change: > > xfs_file_buffered_write( > struct kiocb *iocb, > struct iov_iter *from) > { > struct file *file = iocb->ki_filp; > struct address_space *mapping = file->f_mapping; > struct inode *inode = mapping->host; > struct xfs_inode *ip = XFS_I(inode); > > In what cases does file_inode(iocb->ki_filp) point to a different > inode than iocb->ki_filp->f_mapping->host? The dio write path assumes > that file_inode(iocb->ki_filp) is correct, as do both the buffered > and dio read paths. > > What makes the buffered write path special in that > file_inode(iocb->ki_filp) is not correctly set whilst > iocb->ki_filp->f_mapping->host is? > In the function xfs_file_buffered_write() the code calls the function xfs_ilock(). The xfs_inode pointer that is passed in is iocb->ki_filp->f_mapping->host. The one used in xfs_ilock_iocb is ki_filp->f_inode. After getting the lock, the code in xfs_file_buffered_write calls the function xfs_buffered_write_iomap_begin(). In this function the code calls xfs_ilock() for ki_filp->f_inode in exclusive mode. If I replace the first xfs_ilock() call with xfs_ilock_iocb(), then it looks like I get a deadlock. Am I missing something? I can: - replace the pointer to iocb with pointer to xfs_inode in the function xfs_ilock_iocb() and also pass in the flags value as a parameter. or - create function xfs_ilock_inode(), which xfs_ilock_iocb() calls. The existing calls will not need to change, only the xfs_ilock in xfs_file_buffered_write() will use xfs_ilock_inode(). > Regardless, if this is a problem, then just pass the XFS inode to > xfs_ilock_iocb() and this is a moot point. > >> However here we need to use iocb->ki_filp->f_mapping->host. >> I'll split off new helper for this in the next version of the patch. > > We don't need a new helper here, either. > > Cheers, > > Dave.
On Mon, May 02, 2022 at 02:21:17PM -0700, Stefan Roesch wrote: > > > On 4/28/22 2:54 PM, Dave Chinner wrote: > > On Thu, Apr 28, 2022 at 12:58:59PM -0700, Stefan Roesch wrote: > >> > >> > >> On 4/26/22 3:56 PM, Dave Chinner wrote: > >>> On Tue, Apr 26, 2022 at 10:43:28AM -0700, Stefan Roesch wrote: > >>>> This adds the async buffered write support to XFS. For async buffered > >>>> write requests, the request will return -EAGAIN if the ilock cannot be > >>>> obtained immediately. > >>>> > >>>> Signed-off-by: Stefan Roesch <shr@fb.com> > >>>> --- > >>>> fs/xfs/xfs_file.c | 10 ++++++---- > >>>> 1 file changed, 6 insertions(+), 4 deletions(-) > >>>> > >>>> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c > >>>> index 6f9da1059e8b..49d54b939502 100644 > >>>> --- a/fs/xfs/xfs_file.c > >>>> +++ b/fs/xfs/xfs_file.c > >>>> @@ -739,12 +739,14 @@ xfs_file_buffered_write( > >>>> bool cleared_space = false; > >>>> int iolock; > >>>> > >>>> - if (iocb->ki_flags & IOCB_NOWAIT) > >>>> - return -EOPNOTSUPP; > >>>> - > >>>> write_retry: > >>>> iolock = XFS_IOLOCK_EXCL; > >>>> - xfs_ilock(ip, iolock); > >>>> + if (iocb->ki_flags & IOCB_NOWAIT) { > >>>> + if (!xfs_ilock_nowait(ip, iolock)) > >>>> + return -EAGAIN; > >>>> + } else { > >>>> + xfs_ilock(ip, iolock); > >>>> + } > >>> > >>> xfs_ilock_iocb(). > >>> > >> > >> The helper xfs_ilock_iocb cannot be used as it hardcoded to use iocb->ki_filp to > >> get a pointer to the xfs_inode. > > > > And the problem with that is? > > > > I mean, look at what xfs_file_buffered_write() does to get the > > xfs_inode 10 lines about that change: > > > > xfs_file_buffered_write( > > struct kiocb *iocb, > > struct iov_iter *from) > > { > > struct file *file = iocb->ki_filp; > > struct address_space *mapping = file->f_mapping; > > struct inode *inode = mapping->host; > > struct xfs_inode *ip = XFS_I(inode); > > > > In what cases does file_inode(iocb->ki_filp) point to a different > > inode than iocb->ki_filp->f_mapping->host? The dio write path assumes > > that file_inode(iocb->ki_filp) is correct, as do both the buffered > > and dio read paths. > > > > What makes the buffered write path special in that > > file_inode(iocb->ki_filp) is not correctly set whilst > > iocb->ki_filp->f_mapping->host is? > > > > In the function xfs_file_buffered_write() the code calls the function > xfs_ilock(). The xfs_inode pointer that is passed in is iocb->ki_filp->f_mapping->host. > The one used in xfs_ilock_iocb is ki_filp->f_inode. > > After getting the lock, the code in xfs_file_buffered_write calls the > function xfs_buffered_write_iomap_begin(). In this function the code > calls xfs_ilock() for ki_filp->f_inode in exclusive mode. > > If I replace the first xfs_ilock() call with xfs_ilock_iocb(), then it looks > like I get a deadlock. > > Am I missing something? Yes. They take different locks. xfs_file_buffered_write() takes the IOLOCK, xfs_buffered_write_iomap_begin() takes the ILOCK.... > I can: > - replace the pointer to iocb with pointer to xfs_inode in the function xfs_ilock_iocb() > and also pass in the flags value as a parameter. > or > - create function xfs_ilock_inode(), which xfs_ilock_iocb() calls. The existing > calls will not need to change, only the xfs_ilock in xfs_file_buffered_write() > will use xfs_ilock_inode(). You're making this way more complex than it needs to be. As I said: > > Regardless, if this is a problem, then just pass the XFS inode to > > xfs_ilock_iocb() and this is a moot point. Cheers, Dave.
On 5/6/22 2:29 AM, Dave Chinner wrote: > On Mon, May 02, 2022 at 02:21:17PM -0700, Stefan Roesch wrote: >> >> >> On 4/28/22 2:54 PM, Dave Chinner wrote: >>> On Thu, Apr 28, 2022 at 12:58:59PM -0700, Stefan Roesch wrote: >>>> >>>> >>>> On 4/26/22 3:56 PM, Dave Chinner wrote: >>>>> On Tue, Apr 26, 2022 at 10:43:28AM -0700, Stefan Roesch wrote: >>>>>> This adds the async buffered write support to XFS. For async buffered >>>>>> write requests, the request will return -EAGAIN if the ilock cannot be >>>>>> obtained immediately. >>>>>> >>>>>> Signed-off-by: Stefan Roesch <shr@fb.com> >>>>>> --- >>>>>> fs/xfs/xfs_file.c | 10 ++++++---- >>>>>> 1 file changed, 6 insertions(+), 4 deletions(-) >>>>>> >>>>>> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c >>>>>> index 6f9da1059e8b..49d54b939502 100644 >>>>>> --- a/fs/xfs/xfs_file.c >>>>>> +++ b/fs/xfs/xfs_file.c >>>>>> @@ -739,12 +739,14 @@ xfs_file_buffered_write( >>>>>> bool cleared_space = false; >>>>>> int iolock; >>>>>> >>>>>> - if (iocb->ki_flags & IOCB_NOWAIT) >>>>>> - return -EOPNOTSUPP; >>>>>> - >>>>>> write_retry: >>>>>> iolock = XFS_IOLOCK_EXCL; >>>>>> - xfs_ilock(ip, iolock); >>>>>> + if (iocb->ki_flags & IOCB_NOWAIT) { >>>>>> + if (!xfs_ilock_nowait(ip, iolock)) >>>>>> + return -EAGAIN; >>>>>> + } else { >>>>>> + xfs_ilock(ip, iolock); >>>>>> + } >>>>> >>>>> xfs_ilock_iocb(). >>>>> >>>> >>>> The helper xfs_ilock_iocb cannot be used as it hardcoded to use iocb->ki_filp to >>>> get a pointer to the xfs_inode. >>> >>> And the problem with that is? >>> >>> I mean, look at what xfs_file_buffered_write() does to get the >>> xfs_inode 10 lines about that change: >>> >>> xfs_file_buffered_write( >>> struct kiocb *iocb, >>> struct iov_iter *from) >>> { >>> struct file *file = iocb->ki_filp; >>> struct address_space *mapping = file->f_mapping; >>> struct inode *inode = mapping->host; >>> struct xfs_inode *ip = XFS_I(inode); >>> >>> In what cases does file_inode(iocb->ki_filp) point to a different >>> inode than iocb->ki_filp->f_mapping->host? The dio write path assumes >>> that file_inode(iocb->ki_filp) is correct, as do both the buffered >>> and dio read paths. >>> >>> What makes the buffered write path special in that >>> file_inode(iocb->ki_filp) is not correctly set whilst >>> iocb->ki_filp->f_mapping->host is? >>> >> >> In the function xfs_file_buffered_write() the code calls the function >> xfs_ilock(). The xfs_inode pointer that is passed in is iocb->ki_filp->f_mapping->host. >> The one used in xfs_ilock_iocb is ki_filp->f_inode. >> >> After getting the lock, the code in xfs_file_buffered_write calls the >> function xfs_buffered_write_iomap_begin(). In this function the code >> calls xfs_ilock() for ki_filp->f_inode in exclusive mode. >> >> If I replace the first xfs_ilock() call with xfs_ilock_iocb(), then it looks >> like I get a deadlock. >> >> Am I missing something? > > Yes. They take different locks. xfs_file_buffered_write() takes the > IOLOCK, xfs_buffered_write_iomap_begin() takes the ILOCK.... > Thanks for the clarification. >> I can: >> - replace the pointer to iocb with pointer to xfs_inode in the function xfs_ilock_iocb() >> and also pass in the flags value as a parameter. >> or >> - create function xfs_ilock_inode(), which xfs_ilock_iocb() calls. The existing >> calls will not need to change, only the xfs_ilock in xfs_file_buffered_write() >> will use xfs_ilock_inode(). > > You're making this way more complex than it needs to be. As I said: > >>> Regardless, if this is a problem, then just pass the XFS inode to >>> xfs_ilock_iocb() and this is a moot point. > The function xfs_ilock_iocb() is expecting a pointer to the data structure kiocb, not a pointer to xfs_inode. I don't see how that's possible without changing the signature of xfs_ilock_iocb(). Do you want to invoke xfs_ilock_nowait() directly()? > Cheers, > > Dave.
On Mon, May 09, 2022 at 12:32:59PM -0700, Stefan Roesch wrote: > On 5/6/22 2:29 AM, Dave Chinner wrote: > > On Mon, May 02, 2022 at 02:21:17PM -0700, Stefan Roesch wrote: > >> On 4/28/22 2:54 PM, Dave Chinner wrote: > >>> On Thu, Apr 28, 2022 at 12:58:59PM -0700, Stefan Roesch wrote: > >> - replace the pointer to iocb with pointer to xfs_inode in the function xfs_ilock_iocb() > >> and also pass in the flags value as a parameter. > >> or > >> - create function xfs_ilock_inode(), which xfs_ilock_iocb() calls. The existing > >> calls will not need to change, only the xfs_ilock in xfs_file_buffered_write() > >> will use xfs_ilock_inode(). > > > > You're making this way more complex than it needs to be. As I said: > > > >>> Regardless, if this is a problem, then just pass the XFS inode to > >>> xfs_ilock_iocb() and this is a moot point. > > > > The function xfs_ilock_iocb() is expecting a pointer to the data structure kiocb, not > a pointer to xfs_inode. I don't see how that's possible without changing the signature > of xfs_ilock_iocb(). For the *third time*: pass the xfs_inode to xfs_ilock_iocb() and update all the callers to do the same thing. -Dave.
On Tue, May 10, 2022 at 09:24:25AM +1000, Dave Chinner wrote: > On Mon, May 09, 2022 at 12:32:59PM -0700, Stefan Roesch wrote: > > On 5/6/22 2:29 AM, Dave Chinner wrote: > > > On Mon, May 02, 2022 at 02:21:17PM -0700, Stefan Roesch wrote: > > >> On 4/28/22 2:54 PM, Dave Chinner wrote: > > >>> On Thu, Apr 28, 2022 at 12:58:59PM -0700, Stefan Roesch wrote: > > >> - replace the pointer to iocb with pointer to xfs_inode in the function xfs_ilock_iocb() > > >> and also pass in the flags value as a parameter. > > >> or > > >> - create function xfs_ilock_inode(), which xfs_ilock_iocb() calls. The existing > > >> calls will not need to change, only the xfs_ilock in xfs_file_buffered_write() > > >> will use xfs_ilock_inode(). > > > > > > You're making this way more complex than it needs to be. As I said: > > > > > >>> Regardless, if this is a problem, then just pass the XFS inode to > > >>> xfs_ilock_iocb() and this is a moot point. > > > > > > > The function xfs_ilock_iocb() is expecting a pointer to the data structure kiocb, not > > a pointer to xfs_inode. I don't see how that's possible without changing the signature > > of xfs_ilock_iocb(). > > For the *third time*: pass the xfs_inode to xfs_ilock_iocb() and > update all the callers to do the same thing. I still don't understand why /any/ of this is necessary. When does iocb->ki_filp->f_inode != iocb->ki_filp->f_mapping->host? --D > -Dave. > -- > Dave Chinner > david@fromorbit.com
On Mon, May 09, 2022 at 04:44:24PM -0700, Darrick J. Wong wrote: > On Tue, May 10, 2022 at 09:24:25AM +1000, Dave Chinner wrote: > > On Mon, May 09, 2022 at 12:32:59PM -0700, Stefan Roesch wrote: > > > On 5/6/22 2:29 AM, Dave Chinner wrote: > > > > On Mon, May 02, 2022 at 02:21:17PM -0700, Stefan Roesch wrote: > > > >> On 4/28/22 2:54 PM, Dave Chinner wrote: > > > >>> On Thu, Apr 28, 2022 at 12:58:59PM -0700, Stefan Roesch wrote: > > > >> - replace the pointer to iocb with pointer to xfs_inode in the function xfs_ilock_iocb() > > > >> and also pass in the flags value as a parameter. > > > >> or > > > >> - create function xfs_ilock_inode(), which xfs_ilock_iocb() calls. The existing > > > >> calls will not need to change, only the xfs_ilock in xfs_file_buffered_write() > > > >> will use xfs_ilock_inode(). > > > > > > > > You're making this way more complex than it needs to be. As I said: > > > > > > > >>> Regardless, if this is a problem, then just pass the XFS inode to > > > >>> xfs_ilock_iocb() and this is a moot point. > > > > > > > > > > The function xfs_ilock_iocb() is expecting a pointer to the data structure kiocb, not > > > a pointer to xfs_inode. I don't see how that's possible without changing the signature > > > of xfs_ilock_iocb(). > > > > For the *third time*: pass the xfs_inode to xfs_ilock_iocb() and > > update all the callers to do the same thing. > > I still don't understand why /any/ of this is necessary. When does > iocb->ki_filp->f_inode != iocb->ki_filp->f_mapping->host? I already asked that question because I don't know the answer, either. I suspect the answer is "block dev inodes" but that then just raises the question of "how do we get them here?" and I don't know the answer to that, either. I don't have the time to dig into this and I don't expect anyone to just pop up with an answer, either. So in the mean time, we can just ignore it for the purpose of this patch set... Cheers, Dave.
On Tue, May 10, 2022 at 11:12:05AM +1000, Dave Chinner wrote: > > I still don't understand why /any/ of this is necessary. When does > > iocb->ki_filp->f_inode != iocb->ki_filp->f_mapping->host? > > I already asked that question because I don't know the answer, > either. I suspect the answer is "block dev inodes" but that then > just raises the question of "how do we get them here?" and I don't > know the answer to that, either. I don't have the time to dig into > this and I don't expect anyone to just pop up with an answer, > either. So in the mean time, we can just ignore it for the purpose > of this patch set... Weird device nodes (including block device) is the answer. It never happens for a normal file system file struct that we'd see in XFS.
On Mon, May 09, 2022 at 11:47:59PM -0700, Christoph Hellwig wrote: > On Tue, May 10, 2022 at 11:12:05AM +1000, Dave Chinner wrote: > > > I still don't understand why /any/ of this is necessary. When does > > > iocb->ki_filp->f_inode != iocb->ki_filp->f_mapping->host? > > > > I already asked that question because I don't know the answer, > > either. I suspect the answer is "block dev inodes" but that then > > just raises the question of "how do we get them here?" and I don't > > know the answer to that, either. I don't have the time to dig into > > this and I don't expect anyone to just pop up with an answer, > > either. So in the mean time, we can just ignore it for the purpose > > of this patch set... > > Weird device nodes (including block device) is the answer. It never > happens for a normal file system file struct that we'd see in XFS. Ok, so we can just use XFS_I(file_inode(iocb->ki_filp)) then and we don't need to pass the xfs_inode at all. We probably should convert the rest of the io path to do this as well so we don't end up forgetting this again... Cheers, Dave.
On Mon, May 16, 2022 at 12:24:30PM +1000, Dave Chinner wrote: > Ok, so we can just use XFS_I(file_inode(iocb->ki_filp)) then and we > don't need to pass the xfs_inode at all. We probably should convert > the rest of the io path to do this as well so we don't end up > forgetting this again... Yes.
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c index 6f9da1059e8b..49d54b939502 100644 --- a/fs/xfs/xfs_file.c +++ b/fs/xfs/xfs_file.c @@ -739,12 +739,14 @@ xfs_file_buffered_write( bool cleared_space = false; int iolock; - if (iocb->ki_flags & IOCB_NOWAIT) - return -EOPNOTSUPP; - write_retry: iolock = XFS_IOLOCK_EXCL; - xfs_ilock(ip, iolock); + if (iocb->ki_flags & IOCB_NOWAIT) { + if (!xfs_ilock_nowait(ip, iolock)) + return -EAGAIN; + } else { + xfs_ilock(ip, iolock); + } ret = xfs_file_write_checks(iocb, from, &iolock); if (ret)
This adds the async buffered write support to XFS. For async buffered write requests, the request will return -EAGAIN if the ilock cannot be obtained immediately. Signed-off-by: Stefan Roesch <shr@fb.com> --- fs/xfs/xfs_file.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)