From: Christoph Hellwig <hch@lst.de>
To: John Garry <john.g.garry@oracle.com>
Cc: axboe@kernel.dk, kbusch@kernel.org, hch@lst.de, sagi@grimberg.me,
jejb@linux.ibm.com, martin.petersen@oracle.com,
djwong@kernel.org, viro@zeniv.linux.org.uk, brauner@kernel.org,
dchinner@redhat.com, jack@suse.cz, linux-block@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-nvme@lists.infradead.org,
linux-fsdevel@vger.kernel.org, tytso@mit.edu, jbongio@google.com,
linux-scsi@vger.kernel.org, ojaswin@linux.ibm.com,
linux-aio@kvack.org, linux-btrfs@vger.kernel.org,
io-uring@vger.kernel.org, nilay@linux.ibm.com,
ritesh.list@gmail.com, willy@infradead.org,
Prasad Singamsetty <prasad.singamsetty@oracle.com>
Subject: Re: [PATCH v7 2/9] fs: Initial atomic write support
Date: Wed, 5 Jun 2024 10:30:15 +0200 [thread overview]
Message-ID: <20240605083015.GA20984@lst.de> (raw)
In-Reply-To: <20240602140912.970947-3-john.g.garry@oracle.com>
Highlevel question: in a lot of the discussions we've used the
term "untorn writes" instead, which feels better than atomic to
me as atomic is a highly overloaded term. Should we switch the
naming to that?
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 0283cf366c2a..6cb67882bcfd 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -45,6 +45,7 @@
> #include <linux/slab.h>
> #include <linux/maple_tree.h>
> #include <linux/rw_hint.h>
> +#include <linux/uio.h>
fs.h is included almost everywhere, so if we can avoid pulling in
even more dependencies that would be great.
It seems like it is pulled in just for this helper:
> +static inline
> +bool generic_atomic_write_valid(loff_t pos, struct iov_iter *iter)
> +{
> + size_t len = iov_iter_count(iter);
> +
> + if (!iter_is_ubuf(iter))
> + return false;
> +
> + if (!is_power_of_2(len))
> + return false;
> +
> + if (!IS_ALIGNED(pos, len))
> + return false;
> +
> + return true;
> +}
should that just go to uio.h instead, or move out of line?
Also the return type formatting is wrong, the two normal styles are
either:
static inline bool generic_atomic_write_valid(loff_t pos,
struct iov_iter *iter)
or:
static inline bool
generic_atomic_write_valid(loff_t pos, struct iov_iter *iter)
(and while I'm at nitpicking, passing the pos before the iter
feels weird)
Last but not least: if READ/WRITE is passed to kiocb_set_rw_flags,
it should probably set IOCB_WRITE as well? That might be a worthwile
prep patch on it's own.
next prev parent reply other threads:[~2024-06-05 8:30 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-02 14:09 [PATCH v7 0/9] block atomic writes John Garry
2024-06-02 14:09 ` [PATCH v7 1/9] block: Pass blk_queue_get_max_sectors() a request pointer John Garry
2024-06-02 14:09 ` [PATCH v7 2/9] fs: Initial atomic write support John Garry
2024-06-05 8:30 ` Christoph Hellwig [this message]
2024-06-05 10:48 ` John Garry
2024-06-06 5:41 ` Christoph Hellwig
2024-06-06 6:38 ` John Garry
2024-06-06 12:47 ` Dave Chinner
2024-06-02 14:09 ` [PATCH v7 3/9] fs: Add initial atomic write support info to statx John Garry
2024-06-02 14:09 ` [PATCH v7 4/9] block: Add core atomic write support John Garry
2024-06-03 9:26 ` Hannes Reinecke
2024-06-03 11:38 ` John Garry
2024-06-03 12:31 ` Hannes Reinecke
2024-06-03 13:29 ` John Garry
2024-06-05 8:32 ` Christoph Hellwig
2024-06-05 11:21 ` John Garry
2024-06-06 5:44 ` Christoph Hellwig
2024-06-05 8:31 ` Christoph Hellwig
2024-06-02 14:09 ` [PATCH v7 5/9] block: Add atomic write support for statx John Garry
2024-06-02 14:09 ` [PATCH v7 6/9] block: Add fops atomic write support John Garry
2024-06-02 14:09 ` [PATCH v7 7/9] scsi: sd: Atomic " John Garry
2024-06-02 14:09 ` [PATCH v7 8/9] scsi: scsi_debug: " John Garry
2024-06-02 14:09 ` [PATCH v7 9/9] nvme: " John Garry
2024-06-07 6:16 ` [PATCH v7 0/9] block atomic writes John Garry
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=20240605083015.GA20984@lst.de \
--to=hch@lst.de \
--cc=axboe@kernel.dk \
--cc=brauner@kernel.org \
--cc=dchinner@redhat.com \
--cc=djwong@kernel.org \
--cc=io-uring@vger.kernel.org \
--cc=jack@suse.cz \
--cc=jbongio@google.com \
--cc=jejb@linux.ibm.com \
--cc=john.g.garry@oracle.com \
--cc=kbusch@kernel.org \
--cc=linux-aio@kvack.org \
--cc=linux-block@vger.kernel.org \
--cc=linux-btrfs@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nvme@lists.infradead.org \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=nilay@linux.ibm.com \
--cc=ojaswin@linux.ibm.com \
--cc=prasad.singamsetty@oracle.com \
--cc=ritesh.list@gmail.com \
--cc=sagi@grimberg.me \
--cc=tytso@mit.edu \
--cc=viro@zeniv.linux.org.uk \
--cc=willy@infradead.org \
/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.