Re: [Cluster-devel] [PATCH v6 15/20] md: raid1: check if adding pages to resync bio fails
On Tue, May 30, 2023 at 9:25 PM Christoph Hellwig wrote: > > To me these look like __bio_add_page candidates, but I guess Song > preferred it this way? It'll add a bit pointless boilerplate code, > but I'm ok with that. We had some discussion on this in v2, and decided to keep these assert-like WARN_ON(). Thanks, Song
Re: [Cluster-devel] [PATCH v6 08/20] jfs: logmgr: use __bio_add_page to add single page to bio
Looks good: Reviewed-by: Christoph Hellwig
Re: [Cluster-devel] [PATCH v6 20/20] block: mark bio_add_folio as __must_check
On Tue, May 30, 2023 at 08:49:23AM -0700, Johannes Thumshirn wrote: > +bool __must_check bio_add_folio(struct bio *, struct folio *, size_t len, > size_t off); Please spell out the parameters and avoid the overly long line.
Re: [Cluster-devel] [PATCH v6 19/20] fs: iomap: use __bio_add_folio where possible
On Tue, May 30, 2023 at 08:49:22AM -0700, Johannes Thumshirn wrote: > When the iomap buffered-io code can't add a folio to a bio, it allocates a > new bio and adds the folio to that one. This is done using bio_add_folio(), > but doesn't check for errors. > > As adding a folio to a newly created bio can't fail, use the newly > introduced __bio_add_folio() function. Looks good: Reviewed-by: Christoph Hellwig
Re: [Cluster-devel] [PATCH v6 18/20] block: add __bio_add_folio
On Tue, May 30, 2023 at 08:49:21AM -0700, Johannes Thumshirn wrote: > Just like for bio_add_pages() add a no-fail variant for bio_add_folio(). Can we call this bio_add_folio_nofail? I really regret the __ prefix for bio_add_page these days - it wasn't really intended to be used as widely originally.. > +void __bio_add_folio(struct bio *, struct folio *, size_t len, size_t off); .. and please spell out the parameters.
Re: [Cluster-devel] [PATCH v6 17/20] block: mark bio_add_page as __must_check
> +int __must_check bio_add_page(struct bio *, struct page *, unsigned len, > unsigned off); Please spell out all parameters while you touch this, and also avoid the overly long line.
Re: [Cluster-devel] [PATCH v6 15/20] md: raid1: check if adding pages to resync bio fails
To me these look like __bio_add_page candidates, but I guess Song preferred it this way? It'll add a bit pointless boilerplate code, but I'm ok with that.
Re: [Cluster-devel] [PATCH v6 10/20] zonefs: use __bio_add_page for adding single page to bio
Looks good: Reviewed-by: Christoph Hellwig
Re: [Cluster-devel] [PATCH v6 14/20] md: raid1: use __bio_add_page for adding single page to bio
Looks good: Reviewed-by: Christoph Hellwig
Re: [Cluster-devel] [PATCH v6 13/20] md: check for failure when adding pages in alloc_behind_master_bio
On Tue, May 30, 2023 at 08:49:16AM -0700, Johannes Thumshirn wrote: > alloc_behind_master_bio() can possibly add multiple pages to a bio, but it > is not checking for the return value of bio_add_page() if adding really > succeeded. > > Check if the page adding succeeded and if not bail out. > > Reviewed-by: Damien Le Moal > Signed-off-by: Johannes Thumshirn Looks good: Reviewed-by: Christoph Hellwig
Re: [Cluster-devel] [PATCH v6 12/20] floppy: use __bio_add_page for adding single page to bio
Looks good: Reviewed-by: Christoph Hellwig
Re: [Cluster-devel] [PATCH v6 11/20] zram: use __bio_add_page for adding single page to bio
Looks good: Reviewed-by: Christoph Hellwig
Re: [Cluster-devel] [PATCH v6 09/20] gfs2: use __bio_add_page for adding single page to bio
Looks good: Reviewed-by: Christoph Hellwig
Re: [Cluster-devel] [PATCH v6 07/20] md: raid5: use __bio_add_page to add single page to new bio
Looks good: Reviewed-by: Christoph Hellwig
Re: [Cluster-devel] [PATCH v6 06/20] md: raid5-log: use __bio_add_page to add single page
Looks good: Reviewed-by: Christoph Hellwig
Re: [Cluster-devel] [PATCH v6 04/20] fs: buffer: use __bio_add_page to add single page to bio
Looks good: Reviewed-by: Christoph Hellwig
Re: [Cluster-devel] [PATCH v6 05/20] md: use __bio_add_page to add single page
Looks good: Reviewed-by: Christoph Hellwig
Re: [Cluster-devel] [PATCH v6 01/20] swap: use __bio_add_page to add page to bio
Looks good: Reviewed-by: Christoph Hellwig
Re: [Cluster-devel] [PATCH v6 04/20] fs: buffer: use __bio_add_page to add single page to bio
On 5/30/23 23:49, Johannes Thumshirn wrote: The buffer_head submission code uses bio_add_page() to add a page to a newly created bio. bio_add_page() can fail, but the return value is never checked. Use __bio_add_page() as adding a single page to a newly created bio is guaranteed to succeed. This brings us a step closer to marking bio_add_page() as __must_check. Reviewed-by: Damien Le Moal Signed-off-by: Johannes Thumshirn Reviewed-by: Gou Hao --- fs/buffer.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fs/buffer.c b/fs/buffer.c index a7fc561758b1..63da30ce946a 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -2760,8 +2760,7 @@ static void submit_bh_wbc(blk_opf_t opf, struct buffer_head *bh, bio->bi_iter.bi_sector = bh->b_blocknr * (bh->b_size >> 9); - bio_add_page(bio, bh->b_page, bh->b_size, bh_offset(bh)); - BUG_ON(bio->bi_iter.bi_size != bh->b_size); + __bio_add_page(bio, bh->b_page, bh->b_size, bh_offset(bh)); bio->bi_end_io = end_bio_bh_io_sync; bio->bi_private = bh;
Re: [Cluster-devel] [PATCH v6 13/20] md: check for failure when adding pages in alloc_behind_master_bio
On Tue, May 30, 2023 at 8:50 AM Johannes Thumshirn
wrote:
>
> alloc_behind_master_bio() can possibly add multiple pages to a bio, but it
> is not checking for the return value of bio_add_page() if adding really
> succeeded.
>
> Check if the page adding succeeded and if not bail out.
>
> Reviewed-by: Damien Le Moal
> Signed-off-by: Johannes Thumshirn
Acked-by: Song Liu
> ---
> drivers/md/raid1.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index 68a9e2d9985b..8283ef177f6c 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -1147,7 +1147,10 @@ static void alloc_behind_master_bio(struct r1bio
> *r1_bio,
> if (unlikely(!page))
> goto free_pages;
>
> - bio_add_page(behind_bio, page, len, 0);
> + if (!bio_add_page(behind_bio, page, len, 0)) {
> + free_page(page);
> + goto free_pages;
> + }
>
> size -= len;
> i++;
> --
> 2.40.1
>
Re: [Cluster-devel] [PATCH v5 16/20] dm-crypt: check if adding pages to clone bio fails
On Tue, 30 May 2023, Mike Snitzer wrote:
> On Tue, May 30 2023 at 11:13P -0400,
> Mikulas Patocka wrote:
>
> > Hi
> >
> > I nack this. This just adds code that can't ever be executed.
> >
> > dm-crypt already allocates enough entries in the vector (see "unsigned int
> > nr_iovecs = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;"), so bio_add_page can't
> > fail.
> >
> > If you want to add __must_check to bio_add_page, you should change the
> > dm-crypt code to:
> > if (!bio_add_page(clone, page, len, 0)) {
> > WARN(1, "this can't happen");
> > return NULL;
> > }
> > and not write recovery code for a can't-happen case.
>
> Thanks for the review Mikulas. But the proper way forward, in the
> context of this patchset, is to simply change bio_add_page() to
> __bio_add_page()
>
> Subject becomes: "dm crypt: use __bio_add_page to add single page to clone
> bio"
>
> And header can explain that "crypt_alloc_buffer() already allocates
> enough entries in the clone bio's vector, so bio_add_page can't fail".
>
> Mike
Yes, __bio_add_page would look nicer. But bio_add_page can merge adjacent
pages into a single bvec entry and __bio_add_page can't (I don't know how
often the merging happens or what is the performance implication of
non-merging).
I think that for the next merge window, we can apply this patch:
https://listman.redhat.com/archives/dm-devel/2023-May/054046.html
which makes this discussion irrelevant. (you can change bio_add_page to
__bio_add_page in it)
Mikukas
Re: [Cluster-devel] [PATCHv2 dlm/next] fs: dlm: avoid F_SETLKW plock op lookup collisions
On Tue, May 30, 2023 at 4:08 PM Alexander Aring wrote: > Hi, > > On Tue, May 30, 2023 at 7:01 AM Andreas Gruenbacher > wrote: > > > > On Tue, May 30, 2023 at 12:19 AM Alexander Aring > > wrote: > > > Hi, > > > > > > On Thu, May 25, 2023 at 11:02 AM Andreas Gruenbacher > > > wrote: > > > > > > > > On Wed, May 24, 2023 at 6:02 PM Alexander Aring > > > > wrote: > > > > > This patch fixes a possible plock op collisions when using F_SETLKW > > > > > lock > > > > > requests and fsid, number and owner are not enough to identify a > > > > > result > > > > > for a pending request. The ltp testcases [0] and [1] are examples when > > > > > this is not enough in case of using classic posix locks with threads > > > > > and > > > > > open filedescriptor posix locks. > > > > > > > > > > The idea to fix the issue here is to place all lock request in order. > > > > > In > > > > > case of non F_SETLKW lock request (indicated if wait is set or not) > > > > > the > > > > > lock requests are ordered inside the recv_list. If a result comes back > > > > > the right plock op can be found by the first plock_op in recv_list > > > > > which > > > > > has not info.wait set. This can be done only by non F_SETLKW plock > > > > > ops as > > > > > dlm_controld always reads a specific plock op (list_move_tail() from > > > > > send_list to recv_mlist) and write the result immediately back. > > > > > > > > > > This behaviour is for F_SETLKW not possible as multiple waiters can be > > > > > get a result back in an random order. To avoid a collisions in cases > > > > > like [0] or [1] this patch adds more fields to compare the plock > > > > > operations as the lock request is the same. This is also being made in > > > > > NFS to find an result for an asynchronous F_SETLKW lock request > > > > > [2][3]. We > > > > > still can't find the exact lock request for a specific result if the > > > > > lock request is the same, but if this is the case we don't care the > > > > > order how the identical lock requests get their result back to grant > > > > > the > > > > > lock. > > > > > > > > When the recv_list contains multiple indistinguishable requests, this > > > > can only be because they originated from multiple threads of the same > > > > process. In that case, I agree that it doesn't matter which of those > > > > requests we "complete" in dev_write() as long as we only complete one > > > > request. We do need to compare the additional request fields in > > > > dev_write() to find a suitable request, so that makes sense as well. > > > > We need to compare all of the fields that identify a request (optype, > > > > ex, wait, pid, nodeid, fsid, number, start, end, owner) to find the > > > > "right" request (or in case there is more than one identical request, > > > > a "suitable" request). > > > > > > > > > > In my "definition" why this works is as you said the "identical > > > request". There is a more deeper definition of "when is a request > > > identical" and in my opinion it is here as: "A request A is identical > > > to request B when they get granted under the same 'time'" which is all > > > the fields you mentioned. > > > > > > Even with cancellation (F_SETLKW only) it does not matter which > > > "identical request" you cancel because the kernel and user > > > (dlm_controld) makes no relation between a lock request instance. You > > > need to have at least the same amount of "results" coming back from > > > user space as the amount you are waiting for a result for the same > > > "identical request". > > > > That's not incorrect per se, but cancellations create an additional > > difficulty: they can either succeed or fail. To indicate that a > > cancellation has succeeded, a new type of message can be introduced > > (say, "CANCELLED"), and it's obvious that a CANCELLED message can only > > belong to a locking request that is being cancelled. When cancelling a > > locking request fails, the kernel will see a "locking request granted" > > message though, and when multiple identical locking requests are > > queued and only some of them have been cancelled, it won't be obvious > > which locking request a "locking request granted" message should be > > assigned to anymore. You really don't want to mix things up in that > > case. > > > > This complication makes it a whole lot more difficult to reason about > > the correctness of the code. All that complexity is avoidable by > > sticking with a fixed mapping of requests and replies (i.e., a unique > > request identifier). > > > > To put it differently, you can shoot yourself in the foot and still > > hop along on the other leg, but it may not be the best of all possible > > ideas. > > > > It makes things more complicated, I agree and the reason why this > works now is because there are a lot of "dependencies". I would love > to have an unique identifier to make it possible that we can follow an > instance handle of the original lock request. > > * an unique identifier which also
Re: [Cluster-devel] [PATCH v6 16/20] dm-crypt: check if adding pages to clone bio fails
On 30.05.23 18:10, Mike Snitzer wrote: > On Tue, May 30 2023 at 11:49P -0400, > Johannes Thumshirn wrote: > >> Check if adding pages to clone bio fails and if it does retry with >> reclaim. This mirrors the behaviour of page allocation in >> crypt_alloc_buffer(). > > Nope. > >> This way we can mark bio_add_pages as __must_check. >> >> Reviewed-by: Damien Le Moal >> Signed-off-by: Johannes Thumshirn > > The above patch header doesn't reflect the code. > > I also think __bio_add_page should be used, like my racey reply to > Mikulas vs your v6 patchbomb said, please see: > https://listman.redhat.com/archives/dm-devel/2023-May/054388.html Yep that mail was racing with my send of v6. I can send out a v7 of the series tomorrow or just that one patch updated. Whatever Jens preferes.
Re: [Cluster-devel] [PATCH v6 16/20] dm-crypt: check if adding pages to clone bio fails
On Tue, May 30 2023 at 11:49P -0400,
Johannes Thumshirn wrote:
> Check if adding pages to clone bio fails and if it does retry with
> reclaim. This mirrors the behaviour of page allocation in
> crypt_alloc_buffer().
Nope.
> This way we can mark bio_add_pages as __must_check.
>
> Reviewed-by: Damien Le Moal
> Signed-off-by: Johannes Thumshirn
The above patch header doesn't reflect the code.
I also think __bio_add_page should be used, like my racey reply to
Mikulas vs your v6 patchbomb said, please see:
https://listman.redhat.com/archives/dm-devel/2023-May/054388.html
Thanks,
Mike
> ---
> drivers/md/dm-crypt.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> index 8b47b913ee83..0dd231e61757 100644
> --- a/drivers/md/dm-crypt.c
> +++ b/drivers/md/dm-crypt.c
> @@ -1693,7 +1693,10 @@ static struct bio *crypt_alloc_buffer(struct
> dm_crypt_io *io, unsigned int size)
>
> len = (remaining_size > PAGE_SIZE) ? PAGE_SIZE : remaining_size;
>
> - bio_add_page(clone, page, len, 0);
> + if (!bio_add_page(clone, page, len, 0)) {
> + WARN_ONCE(1, "Adding page to bio failed\n");
> + return NULL;
> + }
>
> remaining_size -= len;
> }
> --
> 2.40.1
>
> --
> dm-devel mailing list
> dm-de...@redhat.com
> https://listman.redhat.com/mailman/listinfo/dm-devel
>
Re: [Cluster-devel] [PATCH v6 20/20] block: mark bio_add_folio as __must_check
On Tue, May 30, 2023 at 08:49:23AM -0700, Johannes Thumshirn wrote: > Now that all callers of bio_add_folio() check the return value, mark it as > __must_check. > > Signed-off-by: Johannes Thumshirn Reviewed-by: Matthew Wilcox (Oracle)
Re: [Cluster-devel] [PATCH v6 19/20] fs: iomap: use __bio_add_folio where possible
On Tue, May 30, 2023 at 08:49:22AM -0700, Johannes Thumshirn wrote: > When the iomap buffered-io code can't add a folio to a bio, it allocates a > new bio and adds the folio to that one. This is done using bio_add_folio(), > but doesn't check for errors. > > As adding a folio to a newly created bio can't fail, use the newly > introduced __bio_add_folio() function. > > Signed-off-by: Johannes Thumshirn Reviewed-by: Matthew Wilcox (Oracle)
Re: [Cluster-devel] [PATCH v6 18/20] block: add __bio_add_folio
On Tue, May 30, 2023 at 08:49:21AM -0700, Johannes Thumshirn wrote: > Just like for bio_add_pages() add a no-fail variant for bio_add_folio(). > > Signed-off-by: Johannes Thumshirn Reviewed-by: Matthew Wilcox (Oracle)
Re: [Cluster-devel] [PATCH v5 16/20] dm-crypt: check if adding pages to clone bio fails
On Tue, May 30 2023 at 11:13P -0400,
Mikulas Patocka wrote:
>
>
> On Tue, 2 May 2023, Johannes Thumshirn wrote:
>
> > Check if adding pages to clone bio fails and if it does retry with
> > reclaim. This mirrors the behaviour of page allocation in
> > crypt_alloc_buffer().
> >
> > This way we can mark bio_add_pages as __must_check.
> >
> > Reviewed-by: Damien Le Moal
> > Signed-off-by: Johannes Thumshirn
> > ---
> > drivers/md/dm-crypt.c | 9 -
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> > index 8b47b913ee83..b234dc089cee 100644
> > --- a/drivers/md/dm-crypt.c
> > +++ b/drivers/md/dm-crypt.c
> > @@ -1693,7 +1693,14 @@ static struct bio *crypt_alloc_buffer(struct
> > dm_crypt_io *io, unsigned int size)
> >
> > len = (remaining_size > PAGE_SIZE) ? PAGE_SIZE : remaining_size;
> >
> > - bio_add_page(clone, page, len, 0);
> > + if (!bio_add_page(clone, page, len, 0)) {
> > + mempool_free(page, >page_pool);
> > + crypt_free_buffer_pages(cc, clone);
> > + bio_put(clone);
> > + gfp_mask |= __GFP_DIRECT_RECLAIM;
> > + goto retry;
> > +
> > + }
> >
> > remaining_size -= len;
> > }
>
> Hi
>
> I nack this. This just adds code that can't ever be executed.
>
> dm-crypt already allocates enough entries in the vector (see "unsigned int
> nr_iovecs = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;"), so bio_add_page can't
> fail.
>
> If you want to add __must_check to bio_add_page, you should change the
> dm-crypt code to:
> if (!bio_add_page(clone, page, len, 0)) {
> WARN(1, "this can't happen");
> return NULL;
> }
> and not write recovery code for a can't-happen case.
Thanks for the review Mikulas. But the proper way forward, in the
context of this patchset, is to simply change bio_add_page() to
__bio_add_page()
Subject becomes: "dm crypt: use __bio_add_page to add single page to clone bio"
And header can explain that "crypt_alloc_buffer() already allocates
enough entries in the clone bio's vector, so bio_add_page can't fail".
Mike
[Cluster-devel] [PATCH v6 20/20] block: mark bio_add_folio as __must_check
Now that all callers of bio_add_folio() check the return value, mark it as __must_check. Signed-off-by: Johannes Thumshirn --- include/linux/bio.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/bio.h b/include/linux/bio.h index 4232a17e6b10..fef9f3085a02 100644 --- a/include/linux/bio.h +++ b/include/linux/bio.h @@ -466,7 +466,7 @@ void bio_reset(struct bio *bio, struct block_device *bdev, blk_opf_t opf); void bio_chain(struct bio *, struct bio *); int __must_check bio_add_page(struct bio *, struct page *, unsigned len, unsigned off); -bool bio_add_folio(struct bio *, struct folio *, size_t len, size_t off); +bool __must_check bio_add_folio(struct bio *, struct folio *, size_t len, size_t off); extern int bio_add_pc_page(struct request_queue *, struct bio *, struct page *, unsigned int, unsigned int); int bio_add_zone_append_page(struct bio *bio, struct page *page, -- 2.40.1
[Cluster-devel] [PATCH v6 18/20] block: add __bio_add_folio
Just like for bio_add_pages() add a no-fail variant for bio_add_folio().
Signed-off-by: Johannes Thumshirn
---
block/bio.c | 8
include/linux/bio.h | 1 +
2 files changed, 9 insertions(+)
diff --git a/block/bio.c b/block/bio.c
index 043944fd46eb..350c653d4a57 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1138,6 +1138,14 @@ int bio_add_page(struct bio *bio, struct page *page,
}
EXPORT_SYMBOL(bio_add_page);
+void __bio_add_folio(struct bio *bio, struct folio *folio, size_t len,
+size_t off)
+{
+ WARN_ON_ONCE(len > UINT_MAX);
+ WARN_ON_ONCE(off > UINT_MAX);
+ __bio_add_page(bio, >page, len, off);
+}
+
/**
* bio_add_folio - Attempt to add part of a folio to a bio.
* @bio: BIO to add to.
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 5d5b081ee062..4232a17e6b10 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -473,6 +473,7 @@ int bio_add_zone_append_page(struct bio *bio, struct page
*page,
unsigned int len, unsigned int offset);
void __bio_add_page(struct bio *bio, struct page *page,
unsigned int len, unsigned int off);
+void __bio_add_folio(struct bio *, struct folio *, size_t len, size_t off);
int bio_iov_iter_get_pages(struct bio *bio, struct iov_iter *iter);
void bio_iov_bvec_set(struct bio *bio, struct iov_iter *iter);
void __bio_release_pages(struct bio *bio, bool mark_dirty);
--
2.40.1
[Cluster-devel] [PATCH v6 19/20] fs: iomap: use __bio_add_folio where possible
When the iomap buffered-io code can't add a folio to a bio, it allocates a
new bio and adds the folio to that one. This is done using bio_add_folio(),
but doesn't check for errors.
As adding a folio to a newly created bio can't fail, use the newly
introduced __bio_add_folio() function.
Signed-off-by: Johannes Thumshirn
---
fs/iomap/buffered-io.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
index 063133ec77f4..42c5fc0ad329 100644
--- a/fs/iomap/buffered-io.c
+++ b/fs/iomap/buffered-io.c
@@ -312,7 +312,7 @@ static loff_t iomap_readpage_iter(const struct iomap_iter
*iter,
ctx->bio->bi_opf |= REQ_RAHEAD;
ctx->bio->bi_iter.bi_sector = sector;
ctx->bio->bi_end_io = iomap_read_end_io;
- bio_add_folio(ctx->bio, folio, plen, poff);
+ __bio_add_folio(ctx->bio, folio, plen, poff);
}
done:
@@ -539,7 +539,7 @@ static int iomap_read_folio_sync(loff_t block_start, struct
folio *folio,
bio_init(, iomap->bdev, , 1, REQ_OP_READ);
bio.bi_iter.bi_sector = iomap_sector(iomap, block_start);
- bio_add_folio(, folio, plen, poff);
+ __bio_add_folio(, folio, plen, poff);
return submit_bio_wait();
}
@@ -1582,7 +1582,7 @@ iomap_add_to_ioend(struct inode *inode, loff_t pos,
struct folio *folio,
if (!bio_add_folio(wpc->ioend->io_bio, folio, len, poff)) {
wpc->ioend->io_bio = iomap_chain_bio(wpc->ioend->io_bio);
- bio_add_folio(wpc->ioend->io_bio, folio, len, poff);
+ __bio_add_folio(wpc->ioend->io_bio, folio, len, poff);
}
if (iop)
--
2.40.1
[Cluster-devel] [PATCH v6 15/20] md: raid1: check if adding pages to resync bio fails
Check if adding pages to resync bio fails and if bail out.
As the comment above suggests this cannot happen, WARN if it actually
happens.
This way we can mark bio_add_pages as __must_check.
Reviewed-by: Damien Le Moal
Acked-by: Song Liu
Signed-off-by: Johannes Thumshirn
---
drivers/md/raid1-10.c | 11 ++-
drivers/md/raid10.c | 20 ++--
2 files changed, 16 insertions(+), 15 deletions(-)
diff --git a/drivers/md/raid1-10.c b/drivers/md/raid1-10.c
index e61f6cad4e08..cd349e69ed77 100644
--- a/drivers/md/raid1-10.c
+++ b/drivers/md/raid1-10.c
@@ -101,11 +101,12 @@ static void md_bio_reset_resync_pages(struct bio *bio,
struct resync_pages *rp,
struct page *page = resync_fetch_page(rp, idx);
int len = min_t(int, size, PAGE_SIZE);
- /*
-* won't fail because the vec table is big
-* enough to hold all these pages
-*/
- bio_add_page(bio, page, len, 0);
+ if (WARN_ON(!bio_add_page(bio, page, len, 0))) {
+ bio->bi_status = BLK_STS_RESOURCE;
+ bio_endio(bio);
+ return;
+ }
+
size -= len;
} while (idx++ < RESYNC_PAGES && size > 0);
}
diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 4fcfcb350d2b..381c21f7fb06 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -3819,11 +3819,11 @@ static sector_t raid10_sync_request(struct mddev
*mddev, sector_t sector_nr,
for (bio= biolist ; bio ; bio=bio->bi_next) {
struct resync_pages *rp = get_resync_pages(bio);
page = resync_fetch_page(rp, page_idx);
- /*
-* won't fail because the vec table is big enough
-* to hold all these pages
-*/
- bio_add_page(bio, page, len, 0);
+ if (WARN_ON(!bio_add_page(bio, page, len, 0))) {
+ bio->bi_status = BLK_STS_RESOURCE;
+ bio_endio(bio);
+ goto giveup;
+ }
}
nr_sectors += len>>9;
sector_nr += len>>9;
@@ -4997,11 +4997,11 @@ static sector_t reshape_request(struct mddev *mddev,
sector_t sector_nr,
if (len > PAGE_SIZE)
len = PAGE_SIZE;
for (bio = blist; bio ; bio = bio->bi_next) {
- /*
-* won't fail because the vec table is big enough
-* to hold all these pages
-*/
- bio_add_page(bio, page, len, 0);
+ if (WARN_ON(!bio_add_page(bio, page, len, 0))) {
+ bio->bi_status = BLK_STS_RESOURCE;
+ bio_endio(bio);
+ return sectors_done;
+ }
}
sector_nr += len >> 9;
nr_sectors += len >> 9;
--
2.40.1
[Cluster-devel] [PATCH v6 13/20] md: check for failure when adding pages in alloc_behind_master_bio
alloc_behind_master_bio() can possibly add multiple pages to a bio, but it
is not checking for the return value of bio_add_page() if adding really
succeeded.
Check if the page adding succeeded and if not bail out.
Reviewed-by: Damien Le Moal
Signed-off-by: Johannes Thumshirn
---
drivers/md/raid1.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index 68a9e2d9985b..8283ef177f6c 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -1147,7 +1147,10 @@ static void alloc_behind_master_bio(struct r1bio *r1_bio,
if (unlikely(!page))
goto free_pages;
- bio_add_page(behind_bio, page, len, 0);
+ if (!bio_add_page(behind_bio, page, len, 0)) {
+ free_page(page);
+ goto free_pages;
+ }
size -= len;
i++;
--
2.40.1
[Cluster-devel] [PATCH v6 14/20] md: raid1: use __bio_add_page for adding single page to bio
The sync request code uses bio_add_page() to add a page to a newly created bio. bio_add_page() can fail, but the return value is never checked. Use __bio_add_page() as adding a single page to a newly created bio is guaranteed to succeed. This brings us a step closer to marking bio_add_page() as __must_check. Reviewed-by: Damien Le Moal Acked-by: Song Liu Signed-off-by: Johannes Thumshirn --- drivers/md/raid1.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c index 8283ef177f6c..ff89839455ec 100644 --- a/drivers/md/raid1.c +++ b/drivers/md/raid1.c @@ -2917,7 +2917,7 @@ static sector_t raid1_sync_request(struct mddev *mddev, sector_t sector_nr, * won't fail because the vec table is big * enough to hold all these pages */ - bio_add_page(bio, page, len, 0); + __bio_add_page(bio, page, len, 0); } } nr_sectors += len>>9; -- 2.40.1
[Cluster-devel] [PATCH v6 17/20] block: mark bio_add_page as __must_check
Now that all users of bio_add_page check for the return value, mark bio_add_page as __must_check. Reviewed-by: Damien Le Moal Signed-off-by: Johannes Thumshirn --- include/linux/bio.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/bio.h b/include/linux/bio.h index b3e7529ff55e..5d5b081ee062 100644 --- a/include/linux/bio.h +++ b/include/linux/bio.h @@ -465,7 +465,7 @@ extern void bio_uninit(struct bio *); void bio_reset(struct bio *bio, struct block_device *bdev, blk_opf_t opf); void bio_chain(struct bio *, struct bio *); -int bio_add_page(struct bio *, struct page *, unsigned len, unsigned off); +int __must_check bio_add_page(struct bio *, struct page *, unsigned len, unsigned off); bool bio_add_folio(struct bio *, struct folio *, size_t len, size_t off); extern int bio_add_pc_page(struct request_queue *, struct bio *, struct page *, unsigned int, unsigned int); -- 2.40.1
[Cluster-devel] [PATCH v6 12/20] floppy: use __bio_add_page for adding single page to bio
The floppy code uses bio_add_page() to add a page to a newly created bio. bio_add_page() can fail, but the return value is never checked. Use __bio_add_page() as adding a single page to a newly created bio is guaranteed to succeed. This brings us a step closer to marking bio_add_page() as __must_check. Reviewed-by: Damien Le Moal Signed-off-by: Johannes Thumshirn --- drivers/block/floppy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c index cec2c20f5e59..28ec6b442e9c 100644 --- a/drivers/block/floppy.c +++ b/drivers/block/floppy.c @@ -4147,7 +4147,7 @@ static int __floppy_read_block_0(struct block_device *bdev, int drive) cbdata.drive = drive; bio_init(, bdev, _vec, 1, REQ_OP_READ); - bio_add_page(, page, block_size(bdev), 0); + __bio_add_page(, page, block_size(bdev), 0); bio.bi_iter.bi_sector = 0; bio.bi_flags |= (1 << BIO_QUIET); -- 2.40.1
[Cluster-devel] [PATCH v6 07/20] md: raid5: use __bio_add_page to add single page to new bio
The raid5-ppl submission code uses bio_add_page() to add a page to a
newly created bio. bio_add_page() can fail, but the return value is never
checked. For adding consecutive pages, the return is actually checked and
a new bio is allocated if adding the page fails.
Use __bio_add_page() as adding a single page to a newly created bio is
guaranteed to succeed.
This brings us a step closer to marking bio_add_page() as __must_check.
Reviewed-by: Damien Le Moal
Acked-by: Song Liu
Signed-off-by: Johannes Thumshirn
---
drivers/md/raid5-ppl.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/md/raid5-ppl.c b/drivers/md/raid5-ppl.c
index e495939bb3e0..eaea57aee602 100644
--- a/drivers/md/raid5-ppl.c
+++ b/drivers/md/raid5-ppl.c
@@ -465,7 +465,7 @@ static void ppl_submit_iounit(struct ppl_io_unit *io)
bio->bi_end_io = ppl_log_endio;
bio->bi_iter.bi_sector = log->next_io_sector;
- bio_add_page(bio, io->header_page, PAGE_SIZE, 0);
+ __bio_add_page(bio, io->header_page, PAGE_SIZE, 0);
pr_debug("%s: log->current_io_sector: %llu\n", __func__,
(unsigned long long)log->next_io_sector);
@@ -496,7 +496,7 @@ static void ppl_submit_iounit(struct ppl_io_unit *io)
prev->bi_opf, GFP_NOIO,
_conf->bs);
bio->bi_iter.bi_sector = bio_end_sector(prev);
- bio_add_page(bio, sh->ppl_page, PAGE_SIZE, 0);
+ __bio_add_page(bio, sh->ppl_page, PAGE_SIZE, 0);
bio_chain(bio, prev);
ppl_submit_iounit_bio(io, prev);
--
2.40.1
[Cluster-devel] [PATCH v6 16/20] dm-crypt: check if adding pages to clone bio fails
Check if adding pages to clone bio fails and if it does retry with
reclaim. This mirrors the behaviour of page allocation in
crypt_alloc_buffer().
This way we can mark bio_add_pages as __must_check.
Reviewed-by: Damien Le Moal
Signed-off-by: Johannes Thumshirn
---
drivers/md/dm-crypt.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 8b47b913ee83..0dd231e61757 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -1693,7 +1693,10 @@ static struct bio *crypt_alloc_buffer(struct dm_crypt_io
*io, unsigned int size)
len = (remaining_size > PAGE_SIZE) ? PAGE_SIZE : remaining_size;
- bio_add_page(clone, page, len, 0);
+ if (!bio_add_page(clone, page, len, 0)) {
+ WARN_ONCE(1, "Adding page to bio failed\n");
+ return NULL;
+ }
remaining_size -= len;
}
--
2.40.1
[Cluster-devel] [PATCH v6 06/20] md: raid5-log: use __bio_add_page to add single page
The raid5 log metadata submission code uses bio_add_page() to add a page to a newly created bio. bio_add_page() can fail, but the return value is never checked. Use __bio_add_page() as adding a single page to a newly created bio is guaranteed to succeed. This brings us a step closer to marking bio_add_page() as __must_check. Reviewed-by: Damien Le Moal Acked-by: Song Liu Signed-off-by: Johannes Thumshirn --- drivers/md/raid5-cache.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c index 46182b955aef..852b265c5db4 100644 --- a/drivers/md/raid5-cache.c +++ b/drivers/md/raid5-cache.c @@ -792,7 +792,7 @@ static struct r5l_io_unit *r5l_new_meta(struct r5l_log *log) io->current_bio = r5l_bio_alloc(log); io->current_bio->bi_end_io = r5l_log_endio; io->current_bio->bi_private = io; - bio_add_page(io->current_bio, io->meta_page, PAGE_SIZE, 0); + __bio_add_page(io->current_bio, io->meta_page, PAGE_SIZE, 0); r5_reserve_log_entry(log, io); -- 2.40.1
[Cluster-devel] [PATCH v6 11/20] zram: use __bio_add_page for adding single page to bio
The zram writeback code uses bio_add_page() to add a page to a newly created bio. bio_add_page() can fail, but the return value is never checked. Use __bio_add_page() as adding a single page to a newly created bio is guaranteed to succeed. This brings us a step closer to marking bio_add_page() as __must_check. Reviewed-by: Damien Le Moal Reviewed-by: Sergey Senozhatsky Signed-off-by: Johannes Thumshirn --- drivers/block/zram/zram_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c index f6d90f1ba5cf..b86691d2133e 100644 --- a/drivers/block/zram/zram_drv.c +++ b/drivers/block/zram/zram_drv.c @@ -700,7 +700,7 @@ static ssize_t writeback_store(struct device *dev, bio_init(, zram->bdev, _vec, 1, REQ_OP_WRITE | REQ_SYNC); bio.bi_iter.bi_sector = blk_idx * (PAGE_SIZE >> 9); - bio_add_page(, page, PAGE_SIZE, 0); + __bio_add_page(, page, PAGE_SIZE, 0); /* * XXX: A single page IO would be inefficient for write -- 2.40.1
[Cluster-devel] [PATCH v6 09/20] gfs2: use __bio_add_page for adding single page to bio
The GFS2 superblock reading code uses bio_add_page() to add a page to a newly created bio. bio_add_page() can fail, but the return value is never checked. Use __bio_add_page() as adding a single page to a newly created bio is guaranteed to succeed. This brings us a step closer to marking bio_add_page() as __must_check. Reviewed-by: Damien Le Moal Reviewed-by: Andreas Gruenbacher Signed-off-by: Johannes Thumshirn --- fs/gfs2/ops_fstype.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c index 9af9ddb61ca0..cd962985b058 100644 --- a/fs/gfs2/ops_fstype.c +++ b/fs/gfs2/ops_fstype.c @@ -254,7 +254,7 @@ static int gfs2_read_super(struct gfs2_sbd *sdp, sector_t sector, int silent) bio = bio_alloc(sb->s_bdev, 1, REQ_OP_READ | REQ_META, GFP_NOFS); bio->bi_iter.bi_sector = sector * (sb->s_blocksize >> 9); - bio_add_page(bio, page, PAGE_SIZE, 0); + __bio_add_page(bio, page, PAGE_SIZE, 0); bio->bi_end_io = end_bio_io_page; bio->bi_private = page; -- 2.40.1
[Cluster-devel] [PATCH v6 10/20] zonefs: use __bio_add_page for adding single page to bio
The zonefs superblock reading code uses bio_add_page() to add a page to a newly created bio. bio_add_page() can fail, but the return value is never checked. Use __bio_add_page() as adding a single page to a newly created bio is guaranteed to succeed. This brings us a step closer to marking bio_add_page() as __must_check. Acked-by: Damien Le Moal Signed-off-by: Johannes Thumshirn --- fs/zonefs/super.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/zonefs/super.c b/fs/zonefs/super.c index 23b8b299c64e..9350221abfc5 100644 --- a/fs/zonefs/super.c +++ b/fs/zonefs/super.c @@ -1128,7 +1128,7 @@ static int zonefs_read_super(struct super_block *sb) bio_init(, sb->s_bdev, _vec, 1, REQ_OP_READ); bio.bi_iter.bi_sector = 0; - bio_add_page(, page, PAGE_SIZE, 0); + __bio_add_page(, page, PAGE_SIZE, 0); ret = submit_bio_wait(); if (ret) -- 2.40.1
[Cluster-devel] [PATCH v6 05/20] md: use __bio_add_page to add single page
The md-raid superblock writing code uses bio_add_page() to add a page to a newly created bio. bio_add_page() can fail, but the return value is never checked. Use __bio_add_page() as adding a single page to a newly created bio is guaranteed to succeed. This brings us a step closer to marking bio_add_page() as __must_check. Signed-of_-by: Johannes Thumshirn Reviewed-by: Damien Le Moal Acked-by: Song Liu --- drivers/md/md.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/md/md.c b/drivers/md/md.c index 8e344b4b3444..6a559a7e89c0 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c @@ -938,7 +938,7 @@ void md_super_write(struct mddev *mddev, struct md_rdev *rdev, atomic_inc(>nr_pending); bio->bi_iter.bi_sector = sector; - bio_add_page(bio, page, size, 0); + __bio_add_page(bio, page, size, 0); bio->bi_private = rdev; bio->bi_end_io = super_written; @@ -979,7 +979,7 @@ int sync_page_io(struct md_rdev *rdev, sector_t sector, int size, bio.bi_iter.bi_sector = sector + rdev->new_data_offset; else bio.bi_iter.bi_sector = sector + rdev->data_offset; - bio_add_page(, page, size, 0); + __bio_add_page(, page, size, 0); submit_bio_wait(); -- 2.40.1
[Cluster-devel] [PATCH v6 08/20] jfs: logmgr: use __bio_add_page to add single page to bio
The JFS IO code uses bio_add_page() to add a page to a newly created bio. bio_add_page() can fail, but the return value is never checked. Use __bio_add_page() as adding a single page to a newly created bio is guaranteed to succeed. This brings us a step closer to marking bio_add_page() as __must_check. Reviewed-by: Damien Le Moal Acked-by: Dave Kleikamp Signed-off-by: Johannes Thumshirn --- fs/jfs/jfs_logmgr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/jfs/jfs_logmgr.c b/fs/jfs/jfs_logmgr.c index 695415cbfe98..15c645827dec 100644 --- a/fs/jfs/jfs_logmgr.c +++ b/fs/jfs/jfs_logmgr.c @@ -1974,7 +1974,7 @@ static int lbmRead(struct jfs_log * log, int pn, struct lbuf ** bpp) bio = bio_alloc(log->bdev, 1, REQ_OP_READ, GFP_NOFS); bio->bi_iter.bi_sector = bp->l_blkno << (log->l2bsize - 9); - bio_add_page(bio, bp->l_page, LOGPSIZE, bp->l_offset); + __bio_add_page(bio, bp->l_page, LOGPSIZE, bp->l_offset); BUG_ON(bio->bi_iter.bi_size != LOGPSIZE); bio->bi_end_io = lbmIODone; @@ -2115,7 +2115,7 @@ static void lbmStartIO(struct lbuf * bp) bio = bio_alloc(log->bdev, 1, REQ_OP_WRITE | REQ_SYNC, GFP_NOFS); bio->bi_iter.bi_sector = bp->l_blkno << (log->l2bsize - 9); - bio_add_page(bio, bp->l_page, LOGPSIZE, bp->l_offset); + __bio_add_page(bio, bp->l_page, LOGPSIZE, bp->l_offset); BUG_ON(bio->bi_iter.bi_size != LOGPSIZE); bio->bi_end_io = lbmIODone; -- 2.40.1
[Cluster-devel] [PATCH v6 03/20] dm: dm-zoned: use __bio_add_page for adding single metadata page
dm-zoned uses bio_add_page() for adding a single page to a freshly created metadata bio. Use __bio_add_page() instead as adding a single page to a new bio is always guaranteed to succeed. This brings us a step closer to marking bio_add_page() __must_check Reviewed-by: Damien Le Moal Signed-off-by: Johannes Thumshirn --- drivers/md/dm-zoned-metadata.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/md/dm-zoned-metadata.c b/drivers/md/dm-zoned-metadata.c index 8f0896a6990b..9d3cca8e3dc9 100644 --- a/drivers/md/dm-zoned-metadata.c +++ b/drivers/md/dm-zoned-metadata.c @@ -577,7 +577,7 @@ static struct dmz_mblock *dmz_get_mblock_slow(struct dmz_metadata *zmd, bio->bi_iter.bi_sector = dmz_blk2sect(block); bio->bi_private = mblk; bio->bi_end_io = dmz_mblock_bio_end_io; - bio_add_page(bio, mblk->page, DMZ_BLOCK_SIZE, 0); + __bio_add_page(bio, mblk->page, DMZ_BLOCK_SIZE, 0); submit_bio(bio); return mblk; @@ -728,7 +728,7 @@ static int dmz_write_mblock(struct dmz_metadata *zmd, struct dmz_mblock *mblk, bio->bi_iter.bi_sector = dmz_blk2sect(block); bio->bi_private = mblk; bio->bi_end_io = dmz_mblock_bio_end_io; - bio_add_page(bio, mblk->page, DMZ_BLOCK_SIZE, 0); + __bio_add_page(bio, mblk->page, DMZ_BLOCK_SIZE, 0); submit_bio(bio); return 0; @@ -752,7 +752,7 @@ static int dmz_rdwr_block(struct dmz_dev *dev, enum req_op op, bio = bio_alloc(dev->bdev, 1, op | REQ_SYNC | REQ_META | REQ_PRIO, GFP_NOIO); bio->bi_iter.bi_sector = dmz_blk2sect(block); - bio_add_page(bio, page, DMZ_BLOCK_SIZE, 0); + __bio_add_page(bio, page, DMZ_BLOCK_SIZE, 0); ret = submit_bio_wait(bio); bio_put(bio); -- 2.40.1
[Cluster-devel] [PATCH v6 04/20] fs: buffer: use __bio_add_page to add single page to bio
The buffer_head submission code uses bio_add_page() to add a page to a newly created bio. bio_add_page() can fail, but the return value is never checked. Use __bio_add_page() as adding a single page to a newly created bio is guaranteed to succeed. This brings us a step closer to marking bio_add_page() as __must_check. Reviewed-by: Damien Le Moal Signed-off-by: Johannes Thumshirn --- fs/buffer.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/fs/buffer.c b/fs/buffer.c index a7fc561758b1..63da30ce946a 100644 --- a/fs/buffer.c +++ b/fs/buffer.c @@ -2760,8 +2760,7 @@ static void submit_bh_wbc(blk_opf_t opf, struct buffer_head *bh, bio->bi_iter.bi_sector = bh->b_blocknr * (bh->b_size >> 9); - bio_add_page(bio, bh->b_page, bh->b_size, bh_offset(bh)); - BUG_ON(bio->bi_iter.bi_size != bh->b_size); + __bio_add_page(bio, bh->b_page, bh->b_size, bh_offset(bh)); bio->bi_end_io = end_bio_bh_io_sync; bio->bi_private = bh; -- 2.40.1
[Cluster-devel] [PATCH v6 02/20] drbd: use __bio_add_page to add page to bio
The drbd code only adds a single page to a newly created bio. So use __bio_add_page() to add the page which is guaranteed to succeed in this case. This brings us closer to marking bio_add_page() as __must_check. Reviewed-by: Damien Le Moal Signed-off-by: Johannes Thumshirn --- drivers/block/drbd/drbd_bitmap.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/block/drbd/drbd_bitmap.c b/drivers/block/drbd/drbd_bitmap.c index 6ac8c54b44c7..85ca000a0564 100644 --- a/drivers/block/drbd/drbd_bitmap.c +++ b/drivers/block/drbd/drbd_bitmap.c @@ -1043,9 +1043,7 @@ static void bm_page_io_async(struct drbd_bm_aio_ctx *ctx, int page_nr) __must_ho bio = bio_alloc_bioset(device->ldev->md_bdev, 1, op, GFP_NOIO, _md_io_bio_set); bio->bi_iter.bi_sector = on_disk_sector; - /* bio_add_page of a single page to an empty bio will always succeed, -* according to api. Do we want to assert that? */ - bio_add_page(bio, page, len, 0); + __bio_add_page(bio, page, len, 0); bio->bi_private = ctx; bio->bi_end_io = drbd_bm_endio; -- 2.40.1
[Cluster-devel] [PATCH v6 01/20] swap: use __bio_add_page to add page to bio
The swap code only adds a single page to a newly created bio. So use __bio_add_page() to add the page which is guaranteed to succeed in this case. This brings us closer to marking bio_add_page() as __must_check. Reviewed-by: Damien Le Moal Signed-off-by: Johannes Thumshirn --- mm/page_io.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/mm/page_io.c b/mm/page_io.c index 87b682d18850..684cd3c7b59b 100644 --- a/mm/page_io.c +++ b/mm/page_io.c @@ -338,7 +338,7 @@ static void swap_writepage_bdev_sync(struct page *page, bio_init(, sis->bdev, , 1, REQ_OP_WRITE | REQ_SWAP | wbc_to_write_flags(wbc)); bio.bi_iter.bi_sector = swap_page_sector(page); - bio_add_page(, page, thp_size(page), 0); + __bio_add_page(, page, thp_size(page), 0); bio_associate_blkg_from_page(, page); count_swpout_vm_event(page); @@ -360,7 +360,7 @@ static void swap_writepage_bdev_async(struct page *page, GFP_NOIO); bio->bi_iter.bi_sector = swap_page_sector(page); bio->bi_end_io = end_swap_bio_write; - bio_add_page(bio, page, thp_size(page), 0); + __bio_add_page(bio, page, thp_size(page), 0); bio_associate_blkg_from_page(bio, page); count_swpout_vm_event(page); @@ -468,7 +468,7 @@ static void swap_readpage_bdev_sync(struct page *page, bio_init(, sis->bdev, , 1, REQ_OP_READ); bio.bi_iter.bi_sector = swap_page_sector(page); - bio_add_page(, page, thp_size(page), 0); + __bio_add_page(, page, thp_size(page), 0); /* * Keep this task valid during swap readpage because the oom killer may * attempt to access it in the page fault retry time check. @@ -488,7 +488,7 @@ static void swap_readpage_bdev_async(struct page *page, bio = bio_alloc(sis->bdev, 1, REQ_OP_READ, GFP_KERNEL); bio->bi_iter.bi_sector = swap_page_sector(page); bio->bi_end_io = end_swap_bio_read; - bio_add_page(bio, page, thp_size(page), 0); + __bio_add_page(bio, page, thp_size(page), 0); count_vm_event(PSWPIN); submit_bio(bio); } -- 2.40.1
[Cluster-devel] [PATCH v6 00/20] bio: check return values of bio_add_page
We have two functions for adding a page to a bio, __bio_add_page() which is used to add a single page to a freshly created bio and bio_add_page() which is used to add a page to an existing bio. While __bio_add_page() is expected to succeed, bio_add_page() can fail. This series converts the callers of bio_add_page() which can easily use __bio_add_page() to using it and checks the return of bio_add_page() for callers that don't work on a freshly created bio. Lastly it marks bio_add_page() as __must_check so we don't have to go again and audit all callers. Changes to v5: - Rebased onto latest Linus' master - Removed now superfluous BUG_ON() in fs/buffer.c (Gou) - Removed dead cleanup code in dm-crypt.c (Mikulas) Changes to v4: - Rebased onto latest Linus' master - Dropped already merged patches - Added Sergey's Reviewed-by Changes to v3: - Added __bio_add_folio and use it in iomap (Willy) - Mark bio_add_folio must check (Willy) - s/GFS/GFS2/ (Andreas) Changes to v2: - Removed 'wont fail' comments pointed out by Song Changes to v1: - Removed pointless comment pointed out by Willy - Changed commit messages pointed out by Damien - Colledted Damien's Reviews and Acks Johannes Thumshirn (20): swap: use __bio_add_page to add page to bio drbd: use __bio_add_page to add page to bio dm: dm-zoned: use __bio_add_page for adding single metadata page fs: buffer: use __bio_add_page to add single page to bio md: use __bio_add_page to add single page md: raid5-log: use __bio_add_page to add single page md: raid5: use __bio_add_page to add single page to new bio jfs: logmgr: use __bio_add_page to add single page to bio gfs2: use __bio_add_page for adding single page to bio zonefs: use __bio_add_page for adding single page to bio zram: use __bio_add_page for adding single page to bio floppy: use __bio_add_page for adding single page to bio md: check for failure when adding pages in alloc_behind_master_bio md: raid1: use __bio_add_page for adding single page to bio md: raid1: check if adding pages to resync bio fails dm-crypt: check if adding pages to clone bio fails block: mark bio_add_page as __must_check block: add __bio_add_folio fs: iomap: use __bio_add_folio where possible block: mark bio_add_folio as __must_check block/bio.c | 8 drivers/block/drbd/drbd_bitmap.c | 4 +--- drivers/block/floppy.c | 2 +- drivers/block/zram/zram_drv.c| 2 +- drivers/md/dm-crypt.c| 5 - drivers/md/dm-zoned-metadata.c | 6 +++--- drivers/md/md.c | 4 ++-- drivers/md/raid1-10.c| 11 ++- drivers/md/raid1.c | 7 +-- drivers/md/raid10.c | 20 ++-- drivers/md/raid5-cache.c | 2 +- drivers/md/raid5-ppl.c | 4 ++-- fs/buffer.c | 3 +-- fs/gfs2/ops_fstype.c | 2 +- fs/iomap/buffered-io.c | 6 +++--- fs/jfs/jfs_logmgr.c | 4 ++-- fs/zonefs/super.c| 2 +- include/linux/bio.h | 5 +++-- mm/page_io.c | 8 19 files changed, 59 insertions(+), 46 deletions(-) -- 2.40.1
Re: [Cluster-devel] [PATCH v5 00/20] bio: check return values of bio_add_page
On 5/26/23 12:37 AM, Johannes Thumshirn wrote: > On 24.05.23 17:02, Jens Axboe wrote: >> On 5/2/23 4:19?AM, Johannes Thumshirn wrote: >>> We have two functions for adding a page to a bio, __bio_add_page() which is >>> used to add a single page to a freshly created bio and bio_add_page() which >>> is >>> used to add a page to an existing bio. >>> >>> While __bio_add_page() is expected to succeed, bio_add_page() can fail. >>> >>> This series converts the callers of bio_add_page() which can easily use >>> __bio_add_page() to using it and checks the return of bio_add_page() for >>> callers that don't work on a freshly created bio. >>> >>> Lastly it marks bio_add_page() as __must_check so we don't have to go again >>> and audit all callers. >> >> Looks fine to me, though it would be nice if the fs and dm people could >> give this a quick look. Should not take long, any empty bio addition >> should, by definition, be able to use a non-checked page addition for >> the first page. >> > > I think the FS side is all covered. @Mike could you have a look at the dm > patches? Not the iomap one, that was my main concern. Not that this is tricky stuff, but still... -- Jens Axboe
Re: [Cluster-devel] [dm-devel] [PATCH v5 16/20] dm-crypt: check if adding pages to clone bio fails
On Tue, 2 May 2023, Johannes Thumshirn wrote:
> Check if adding pages to clone bio fails and if it does retry with
> reclaim. This mirrors the behaviour of page allocation in
> crypt_alloc_buffer().
>
> This way we can mark bio_add_pages as __must_check.
>
> Reviewed-by: Damien Le Moal
> Signed-off-by: Johannes Thumshirn
> ---
> drivers/md/dm-crypt.c | 9 -
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> index 8b47b913ee83..b234dc089cee 100644
> --- a/drivers/md/dm-crypt.c
> +++ b/drivers/md/dm-crypt.c
> @@ -1693,7 +1693,14 @@ static struct bio *crypt_alloc_buffer(struct
> dm_crypt_io *io, unsigned int size)
>
> len = (remaining_size > PAGE_SIZE) ? PAGE_SIZE : remaining_size;
>
> - bio_add_page(clone, page, len, 0);
> + if (!bio_add_page(clone, page, len, 0)) {
> + mempool_free(page, >page_pool);
> + crypt_free_buffer_pages(cc, clone);
> + bio_put(clone);
> + gfp_mask |= __GFP_DIRECT_RECLAIM;
> + goto retry;
> +
> + }
>
> remaining_size -= len;
> }
Hi
I nack this. This just adds code that can't ever be executed.
dm-crypt already allocates enough entries in the vector (see "unsigned int
nr_iovecs = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;"), so bio_add_page can't
fail.
If you want to add __must_check to bio_add_page, you should change the
dm-crypt code to:
if (!bio_add_page(clone, page, len, 0)) {
WARN(1, "this can't happen");
return NULL;
}
and not write recovery code for a can't-happen case.
Mikulas
Re: [Cluster-devel] [PATCHv2 dlm/next] fs: dlm: avoid F_SETLKW plock op lookup collisions
Hi, On Tue, May 30, 2023 at 7:01 AM Andreas Gruenbacher wrote: > > On Tue, May 30, 2023 at 12:19 AM Alexander Aring wrote: > > Hi, > > > > On Thu, May 25, 2023 at 11:02 AM Andreas Gruenbacher > > wrote: > > > > > > On Wed, May 24, 2023 at 6:02 PM Alexander Aring > > > wrote: > > > > This patch fixes a possible plock op collisions when using F_SETLKW lock > > > > requests and fsid, number and owner are not enough to identify a result > > > > for a pending request. The ltp testcases [0] and [1] are examples when > > > > this is not enough in case of using classic posix locks with threads and > > > > open filedescriptor posix locks. > > > > > > > > The idea to fix the issue here is to place all lock request in order. In > > > > case of non F_SETLKW lock request (indicated if wait is set or not) the > > > > lock requests are ordered inside the recv_list. If a result comes back > > > > the right plock op can be found by the first plock_op in recv_list which > > > > has not info.wait set. This can be done only by non F_SETLKW plock ops > > > > as > > > > dlm_controld always reads a specific plock op (list_move_tail() from > > > > send_list to recv_mlist) and write the result immediately back. > > > > > > > > This behaviour is for F_SETLKW not possible as multiple waiters can be > > > > get a result back in an random order. To avoid a collisions in cases > > > > like [0] or [1] this patch adds more fields to compare the plock > > > > operations as the lock request is the same. This is also being made in > > > > NFS to find an result for an asynchronous F_SETLKW lock request [2][3]. > > > > We > > > > still can't find the exact lock request for a specific result if the > > > > lock request is the same, but if this is the case we don't care the > > > > order how the identical lock requests get their result back to grant the > > > > lock. > > > > > > When the recv_list contains multiple indistinguishable requests, this > > > can only be because they originated from multiple threads of the same > > > process. In that case, I agree that it doesn't matter which of those > > > requests we "complete" in dev_write() as long as we only complete one > > > request. We do need to compare the additional request fields in > > > dev_write() to find a suitable request, so that makes sense as well. > > > We need to compare all of the fields that identify a request (optype, > > > ex, wait, pid, nodeid, fsid, number, start, end, owner) to find the > > > "right" request (or in case there is more than one identical request, > > > a "suitable" request). > > > > > > > In my "definition" why this works is as you said the "identical > > request". There is a more deeper definition of "when is a request > > identical" and in my opinion it is here as: "A request A is identical > > to request B when they get granted under the same 'time'" which is all > > the fields you mentioned. > > > > Even with cancellation (F_SETLKW only) it does not matter which > > "identical request" you cancel because the kernel and user > > (dlm_controld) makes no relation between a lock request instance. You > > need to have at least the same amount of "results" coming back from > > user space as the amount you are waiting for a result for the same > > "identical request". > > That's not incorrect per se, but cancellations create an additional > difficulty: they can either succeed or fail. To indicate that a > cancellation has succeeded, a new type of message can be introduced > (say, "CANCELLED"), and it's obvious that a CANCELLED message can only > belong to a locking request that is being cancelled. When cancelling a > locking request fails, the kernel will see a "locking request granted" > message though, and when multiple identical locking requests are > queued and only some of them have been cancelled, it won't be obvious > which locking request a "locking request granted" message should be > assigned to anymore. You really don't want to mix things up in that > case. > > This complication makes it a whole lot more difficult to reason about > the correctness of the code. All that complexity is avoidable by > sticking with a fixed mapping of requests and replies (i.e., a unique > request identifier). > > To put it differently, you can shoot yourself in the foot and still > hop along on the other leg, but it may not be the best of all possible > ideas. > It makes things more complicated, I agree and the reason why this works now is because there are a lot of "dependencies". I would love to have an unique identifier to make it possible that we can follow an instance handle of the original lock request. * an unique identifier which also works with the async lock request of lockd case. > > > The above patch description doesn't match the code anymore, and the > > > code doesn't fully revert the recv_list splitting of the previous > > > version. > > > > > > > This isn't a revert. Is it a new patch version, I did drop the > > recv_setlkw_list here,
Re: [Cluster-devel] [PATCHv2 dlm/next] fs: dlm: avoid F_SETLKW plock op lookup collisions
On Tue, May 30, 2023 at 12:19 AM Alexander Aring wrote: > Hi, > > On Thu, May 25, 2023 at 11:02 AM Andreas Gruenbacher > wrote: > > > > On Wed, May 24, 2023 at 6:02 PM Alexander Aring wrote: > > > This patch fixes a possible plock op collisions when using F_SETLKW lock > > > requests and fsid, number and owner are not enough to identify a result > > > for a pending request. The ltp testcases [0] and [1] are examples when > > > this is not enough in case of using classic posix locks with threads and > > > open filedescriptor posix locks. > > > > > > The idea to fix the issue here is to place all lock request in order. In > > > case of non F_SETLKW lock request (indicated if wait is set or not) the > > > lock requests are ordered inside the recv_list. If a result comes back > > > the right plock op can be found by the first plock_op in recv_list which > > > has not info.wait set. This can be done only by non F_SETLKW plock ops as > > > dlm_controld always reads a specific plock op (list_move_tail() from > > > send_list to recv_mlist) and write the result immediately back. > > > > > > This behaviour is for F_SETLKW not possible as multiple waiters can be > > > get a result back in an random order. To avoid a collisions in cases > > > like [0] or [1] this patch adds more fields to compare the plock > > > operations as the lock request is the same. This is also being made in > > > NFS to find an result for an asynchronous F_SETLKW lock request [2][3]. We > > > still can't find the exact lock request for a specific result if the > > > lock request is the same, but if this is the case we don't care the > > > order how the identical lock requests get their result back to grant the > > > lock. > > > > When the recv_list contains multiple indistinguishable requests, this > > can only be because they originated from multiple threads of the same > > process. In that case, I agree that it doesn't matter which of those > > requests we "complete" in dev_write() as long as we only complete one > > request. We do need to compare the additional request fields in > > dev_write() to find a suitable request, so that makes sense as well. > > We need to compare all of the fields that identify a request (optype, > > ex, wait, pid, nodeid, fsid, number, start, end, owner) to find the > > "right" request (or in case there is more than one identical request, > > a "suitable" request). > > > > In my "definition" why this works is as you said the "identical > request". There is a more deeper definition of "when is a request > identical" and in my opinion it is here as: "A request A is identical > to request B when they get granted under the same 'time'" which is all > the fields you mentioned. > > Even with cancellation (F_SETLKW only) it does not matter which > "identical request" you cancel because the kernel and user > (dlm_controld) makes no relation between a lock request instance. You > need to have at least the same amount of "results" coming back from > user space as the amount you are waiting for a result for the same > "identical request". That's not incorrect per se, but cancellations create an additional difficulty: they can either succeed or fail. To indicate that a cancellation has succeeded, a new type of message can be introduced (say, "CANCELLED"), and it's obvious that a CANCELLED message can only belong to a locking request that is being cancelled. When cancelling a locking request fails, the kernel will see a "locking request granted" message though, and when multiple identical locking requests are queued and only some of them have been cancelled, it won't be obvious which locking request a "locking request granted" message should be assigned to anymore. You really don't want to mix things up in that case. This complication makes it a whole lot more difficult to reason about the correctness of the code. All that complexity is avoidable by sticking with a fixed mapping of requests and replies (i.e., a unique request identifier). To put it differently, you can shoot yourself in the foot and still hop along on the other leg, but it may not be the best of all possible ideas. > > The above patch description doesn't match the code anymore, and the > > code doesn't fully revert the recv_list splitting of the previous > > version. > > > > This isn't a revert. Is it a new patch version, I did drop the > recv_setlkw_list here, dropping in means of removing the > recv_setlkw_list and handling everything in the recv_list. Although > there might be a performance impact by splitting the requests in two > lists as we don't need to jump over all F_SETLKW requests. > > > > [0] > > > https://gitlab.com/netcoder/ltp/-/blob/dlm_fcntl_owner_testcase/testcases/kernel/syscalls/fcntl/fcntl40.c > > > [1] > > > https://gitlab.com/netcoder/ltp/-/blob/dlm_fcntl_owner_testcase/testcases/kernel/syscalls/fcntl/fcntl41.c > > > [2] > > >
Re: [Cluster-devel] [PATCH v5 04/20] fs: buffer: use __bio_add_page to add single page to bio
> > The buffer_head submission code uses bio_add_page() to add a page to a > newly created bio. bio_add_page() can fail, but the return value is never > checked. > > Use __bio_add_page() as adding a single page to a newly created bio is > guaranteed to succeed. > > This brings us a step closer to marking bio_add_page() as __must_check. > > Reviewed-by: Damien Le Moal > Signed-off-by: Johannes Thumshirn > --- > fs/buffer.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/buffer.c b/fs/buffer.c > index a7fc561758b1..5abc26d8399d 100644 > --- a/fs/buffer.c > +++ b/fs/buffer.c > @@ -2760,7 +2760,7 @@ static void submit_bh_wbc(blk_opf_t opf, struct > buffer_head *bh, > > bio->bi_iter.bi_sector = bh->b_blocknr * (bh->b_size >> 9); > > - bio_add_page(bio, bh->b_page, bh->b_size, bh_offset(bh)); > + __bio_add_page(bio, bh->b_page, bh->b_size, bh_offset(bh)); > BUG_ON(bio->bi_iter.bi_size != bh->b_size); Can `BUG_ON` be deleted now ? > > bio->bi_end_io = end_bio_bh_io_sync; > -- > 2.40.0 -- thanks, Gou Hao
Re: [Cluster-devel] [PATCH 11/11] fuse: drop redundant arguments to fuse_perform_write
On 5/24/23 08:38, Christoph Hellwig wrote: pos is always equal to iocb->ki_pos, and mapping is always equal to iocb->ki_filp->f_mapping. Signed-off-by: Christoph Hellwig Reviewed-by: Damien Le Moal --- fs/fuse/file.c | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes ReineckeKernel Storage Architect h...@suse.de +49 911 74053 688 SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew Myers, Andrew McDonald, Martje Boudien Moerman
Re: [Cluster-devel] [PATCH 10/11] fuse: update ki_pos in fuse_perform_write
On 5/24/23 08:38, Christoph Hellwig wrote: Both callers of fuse_perform_write need to updated ki_pos, move it into common code. Signed-off-by: Christoph Hellwig Reviewed-by: Damien Le Moal --- fs/fuse/file.c | 25 +++-- 1 file changed, 11 insertions(+), 14 deletions(-) Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes ReineckeKernel Storage Architect h...@suse.de +49 911 74053 688 SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew Myers, Andrew McDonald, Martje Boudien Moerman
Re: [Cluster-devel] [PATCH 09/11] fs: factor out a direct_write_fallback helper
On 5/24/23 08:38, Christoph Hellwig wrote: Add a helper dealing with handling the syncing of a buffered write fallback for direct I/O. Signed-off-by: Christoph Hellwig --- fs/libfs.c | 36 + include/linux/fs.h | 2 ++ mm/filemap.c | 66 +++--- 3 files changed, 53 insertions(+), 51 deletions(-) Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes ReineckeKernel Storage Architect h...@suse.de +49 911 74053 688 SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew Myers, Andrew McDonald, Martje Boudien Moerman
Re: [Cluster-devel] [PATCH 08/11] iomap: use kiocb_write_and_wait and kiocb_invalidate_pages
On 5/24/23 08:38, Christoph Hellwig wrote: Use the common helpers for direct I/O page invalidation instead of open coding the logic. This leads to a slight reordering of checks in __iomap_dio_rw to keep the logic straight. Signed-off-by: Christoph Hellwig Reviewed-by: Damien Le Moal Reviewed-by: Darrick J. Wong --- fs/iomap/direct-io.c | 55 1 file changed, 20 insertions(+), 35 deletions(-) Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes ReineckeKernel Storage Architect h...@suse.de +49 911 74053 688 SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew Myers, Andrew McDonald, Martje Boudien Moerman
Re: [Cluster-devel] [PATCH 07/11] iomap: update ki_pos in iomap_file_buffered_write
On 5/24/23 08:38, Christoph Hellwig wrote: All callers of iomap_file_buffered_write need to updated ki_pos, move it into common code. Signed-off-by: Christoph Hellwig Acked-by: Damien Le Moal Reviewed-by: Darrick J. Wong --- fs/gfs2/file.c | 4 +--- fs/iomap/buffered-io.c | 9 ++--- fs/xfs/xfs_file.c | 2 -- fs/zonefs/file.c | 4 +--- 4 files changed, 8 insertions(+), 11 deletions(-) Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes ReineckeKernel Storage Architect h...@suse.de +49 911 74053 688 SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew Myers, Andrew McDonald, Martje Boudien Moerman
Re: [Cluster-devel] [PATCH 06/11] filemap: add a kiocb_invalidate_post_direct_write helper
On 5/24/23 08:38, Christoph Hellwig wrote: Add a helper to invalidate page cache after a dio write. Signed-off-by: Christoph Hellwig Reviewed-by: Damien Le Moal Acked-by: Darrick J. Wong --- fs/direct-io.c | 10 ++ fs/iomap/direct-io.c| 12 ++-- include/linux/fs.h | 5 - include/linux/pagemap.h | 1 + mm/filemap.c| 37 - 5 files changed, 25 insertions(+), 40 deletions(-) Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes ReineckeKernel Storage Architect h...@suse.de +49 911 74053 688 SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew Myers, Andrew McDonald, Martje Boudien Moerman
Re: [Cluster-devel] [PATCH 05/11] filemap: add a kiocb_invalidate_pages helper
On 5/24/23 08:38, Christoph Hellwig wrote: Factor out a helper that calls filemap_write_and_wait_range and invalidate_inode_pages2_range for the range covered by a write kiocb or returns -EAGAIN if the kiocb is marked as nowait and there would be pages to write or invalidate. Signed-off-by: Christoph Hellwig Reviewed-by: Damien Le Moal Acked-by: Darrick J. Wong --- include/linux/pagemap.h | 1 + mm/filemap.c| 48 - 2 files changed, 29 insertions(+), 20 deletions(-) Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes ReineckeKernel Storage Architect h...@suse.de +49 911 74053 688 SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew Myers, Andrew McDonald, Martje Boudien Moerman
Re: [Cluster-devel] [PATCH 04/11] filemap: add a kiocb_write_and_wait helper
On 5/24/23 08:38, Christoph Hellwig wrote: Factor out a helper that does filemap_write_and_wait_range for the range covered by a read kiocb, or returns -EAGAIN if the kiocb is marked as nowait and there would be pages to write. Signed-off-by: Christoph Hellwig Reviewed-by: Damien Le Moal Acked-by: Darrick J. Wong --- block/fops.c| 18 +++--- include/linux/pagemap.h | 2 ++ mm/filemap.c| 30 ++ 3 files changed, 23 insertions(+), 27 deletions(-) Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes ReineckeKernel Storage Architect h...@suse.de +49 911 74053 688 SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew Myers, Andrew McDonald, Martje Boudien Moerman
Re: [Cluster-devel] [PATCH 03/11] filemap: update ki_pos in generic_perform_write
On 5/24/23 08:38, Christoph Hellwig wrote: All callers of generic_perform_write need to updated ki_pos, move it into common code. Signed-off-by: Christoph Hellwig Reviewed-by: Xiubo Li Reviewed-by: Damien Le Moal Acked-by: Darrick J. Wong --- fs/ceph/file.c | 2 -- fs/ext4/file.c | 9 +++-- fs/f2fs/file.c | 1 - fs/nfs/file.c | 1 - mm/filemap.c | 8 5 files changed, 7 insertions(+), 14 deletions(-) Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes ReineckeKernel Storage Architect h...@suse.de +49 911 74053 688 SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew Myers, Andrew McDonald, Martje Boudien Moerman
Re: [Cluster-devel] [PATCH 02/11] iomap: update ki_pos a little later in iomap_dio_complete
On 5/24/23 08:38, Christoph Hellwig wrote: Move the ki_pos update down a bit to prepare for a better common helper that invalidates pages based of an iocb. Signed-off-by: Christoph Hellwig Reviewed-by: Damien Le Moal Reviewed-by: Darrick J. Wong --- fs/iomap/direct-io.c | 21 +++-- 1 file changed, 11 insertions(+), 10 deletions(-) Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes ReineckeKernel Storage Architect h...@suse.de +49 911 74053 688 SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew Myers, Andrew McDonald, Martje Boudien Moerman
Re: [Cluster-devel] [PATCH 01/11] backing_dev: remove current->backing_dev_info
On 5/24/23 08:38, Christoph Hellwig wrote:
The last user of current->backing_dev_info disappeared in commit
b9b1335e6403 ("remove bdi_congested() and wb_congested() and related
functions"). Remove the field and all assignments to it.
Signed-off-by: Christoph Hellwig
---
fs/btrfs/file.c | 6 +-
fs/ceph/file.c| 4
fs/ext4/file.c| 2 --
fs/f2fs/file.c| 2 --
fs/fuse/file.c| 4
fs/gfs2/file.c| 2 --
fs/nfs/file.c | 5 +
fs/ntfs/file.c| 2 --
fs/ntfs3/file.c | 3 ---
fs/xfs/xfs_file.c | 4
include/linux/sched.h | 3 ---
mm/filemap.c | 3 ---
12 files changed, 2 insertions(+), 38 deletions(-)
Reviewed-by: Hannes Reinecke
Cheers,
Hannes
--
Dr. Hannes ReineckeKernel Storage Architect
h...@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Ivo Totev, Andrew
Myers, Andrew McDonald, Martje Boudien Moerman
