linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [bug report] btrfs: enable nowait async buffered writes
@ 2022-09-23 14:53 Dan Carpenter
  2022-09-26 17:21 ` Stefan Roesch
  0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2022-09-23 14:53 UTC (permalink / raw)
  To: shr; +Cc: linux-btrfs

Hello Stefan Roesch,

The patch 8bf465b1c880: "btrfs: enable nowait async buffered writes"
from Sep 12, 2022, leads to the following Smatch static checker
warning:

	fs/btrfs/file.c:2113 btrfs_do_write_iter()
	warn: refcount leak 'inode->sync_writers.counter': lines='2113'

fs/btrfs/file.c
    2092 ssize_t btrfs_do_write_iter(struct kiocb *iocb, struct iov_iter *from,
    2093                             const struct btrfs_ioctl_encoded_io_args *encoded)
    2094 {
    2095         struct file *file = iocb->ki_filp;
    2096         struct btrfs_inode *inode = BTRFS_I(file_inode(file));
    2097         ssize_t num_written, num_sync;
    2098         const bool sync = iocb_is_dsync(iocb);
    2099 
    2100         /*
    2101          * If the fs flips readonly due to some impossible error, although we
    2102          * have opened a file as writable, we have to stop this write operation
    2103          * to ensure consistency.
    2104          */
    2105         if (BTRFS_FS_ERROR(inode->root->fs_info))
    2106                 return -EROFS;
    2107 
    2108         if (sync)
    2109                 atomic_inc(&inode->sync_writers);
    2110 
    2111         if (encoded) {
    2112                 if (iocb->ki_flags & IOCB_NOWAIT)
--> 2113                         return -EOPNOTSUPP;

This check used to happen before the atomic_inc(&inode->sync_writers);

    2114 
    2115                 num_written = btrfs_encoded_write(iocb, from, encoded);
    2116                 num_sync = encoded->len;
    2117         } else if (iocb->ki_flags & IOCB_DIRECT) {
    2118                 num_written = btrfs_direct_write(iocb, from);
    2119                 num_sync = num_written;
    2120         } else {
    2121                 num_written = btrfs_buffered_write(iocb, from);
    2122                 num_sync = num_written;
    2123         }
    2124 
    2125         btrfs_set_inode_last_sub_trans(inode);
    2126 
    2127         if (num_sync > 0) {
    2128                 num_sync = generic_write_sync(iocb, num_sync);
    2129                 if (num_sync < 0)
    2130                         num_written = num_sync;
    2131         }
    2132 
    2133         if (sync)
    2134                 atomic_dec(&inode->sync_writers);
    2135 
    2136         current->backing_dev_info = NULL;
    2137         return num_written;
    2138 }

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [bug report] btrfs: enable nowait async buffered writes
  2022-09-23 14:53 [bug report] btrfs: enable nowait async buffered writes Dan Carpenter
@ 2022-09-26 17:21 ` Stefan Roesch
  0 siblings, 0 replies; 2+ messages in thread
From: Stefan Roesch @ 2022-09-26 17:21 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: linux-btrfs


On 9/23/22 7:53 AM, Dan Carpenter wrote:
> > 
> Hello Stefan Roesch,
> 
> The patch 8bf465b1c880: "btrfs: enable nowait async buffered writes"
> from Sep 12, 2022, leads to the following Smatch static checker
> warning:
> 
> 	fs/btrfs/file.c:2113 btrfs_do_write_iter()
> 	warn: refcount leak 'inode->sync_writers.counter': lines='2113'
> 
> fs/btrfs/file.c
>     2092 ssize_t btrfs_do_write_iter(struct kiocb *iocb, struct iov_iter *from,
>     2093                             const struct btrfs_ioctl_encoded_io_args *encoded)
>     2094 {
>     2095         struct file *file = iocb->ki_filp;
>     2096         struct btrfs_inode *inode = BTRFS_I(file_inode(file));
>     2097         ssize_t num_written, num_sync;
>     2098         const bool sync = iocb_is_dsync(iocb);
>     2099 
>     2100         /*
>     2101          * If the fs flips readonly due to some impossible error, although we
>     2102          * have opened a file as writable, we have to stop this write operation
>     2103          * to ensure consistency.
>     2104          */
>     2105         if (BTRFS_FS_ERROR(inode->root->fs_info))
>     2106                 return -EROFS;
>     2107 
>     2108         if (sync)
>     2109                 atomic_inc(&inode->sync_writers);
>     2110 
>     2111         if (encoded) {
>     2112                 if (iocb->ki_flags & IOCB_NOWAIT)
> --> 2113                         return -EOPNOTSUPP;
> 
> This check used to happen before the atomic_inc(&inode->sync_writers);
>

Thanks Dan, I sent out a fix for this issue.
--Stefan
 
>     2114 
>     2115                 num_written = btrfs_encoded_write(iocb, from, encoded);
>     2116                 num_sync = encoded->len;
>     2117         } else if (iocb->ki_flags & IOCB_DIRECT) {
>     2118                 num_written = btrfs_direct_write(iocb, from);
>     2119                 num_sync = num_written;
>     2120         } else {
>     2121                 num_written = btrfs_buffered_write(iocb, from);
>     2122                 num_sync = num_written;
>     2123         }
>     2124 
>     2125         btrfs_set_inode_last_sub_trans(inode);
>     2126 
>     2127         if (num_sync > 0) {
>     2128                 num_sync = generic_write_sync(iocb, num_sync);
>     2129                 if (num_sync < 0)
>     2130                         num_written = num_sync;
>     2131         }
>     2132 
>     2133         if (sync)
>     2134                 atomic_dec(&inode->sync_writers);
>     2135 
>     2136         current->backing_dev_info = NULL;
>     2137         return num_written;
>     2138 }
> 
> regards,
> dan carpenter

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-09-26 17:50 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-23 14:53 [bug report] btrfs: enable nowait async buffered writes Dan Carpenter
2022-09-26 17:21 ` Stefan Roesch

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).