Re: [f2fs-dev] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

2022-02-28 Thread Christian König via Linux-f2fs-devel
Am 28.02.22 um 22:13 schrieb James Bottomley: On Mon, 2022-02-28 at 21:56 +0100, Christian König wrote: Am 28.02.22 um 21:42 schrieb James Bottomley: On Mon, 2022-02-28 at 21:07 +0100, Christian König wrote: Am 28.02.22 um 20:56 schrieb Linus Torvalds: On Mon, Feb 28, 2022 at 4:19 AM

Re: [f2fs-dev] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

2022-02-28 Thread Jakub Kicinski
On Mon, 28 Feb 2022 16:41:04 -0800 Linus Torvalds wrote: > So yes, initially my idea had been to just move the iterator entirely > inside the macro. But specifying the type got so ugly that I think > that > > typeof (pos) pos > > trick inside the macro really ends up giving us the best

[f2fs-dev] [PATCH] f2fs-tools: use proper 64bit types for PPC

2022-02-28 Thread Rosen Penev
A specific define is needed. Fixes mostly issues with -Wformat. Signed-off-by: Rosen Penev --- include/f2fs_fs.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h index d236437..412130f 100644 --- a/include/f2fs_fs.h +++ b/include/f2fs_fs.h @@ -41,6

Re: [f2fs-dev] [PATCH 1/6] drivers: usb: remove usage of list iterator past the loop body

2022-02-28 Thread Dan Carpenter
On Mon, Feb 28, 2022 at 10:20:28AM -0800, Joe Perches wrote: > On Mon, 2022-02-28 at 14:24 +0300, Dan Carpenter wrote: > > > a multi-line indent gets curly braces for readability even though > > it's not required by C. And then both sides would get curly braces. > > That's more your personal

Re: [f2fs-dev] [PATCH v2] f2fs: avoid sb_start_intwrite during eviction

2022-02-28 Thread Jaegeuk Kim
1. waiting for f2fs_evict_inode [ 5560.043945] __wait_on_freeing_inode+0xac/0xf0 [ 5560.045540] ? var_wake_function+0x30/0x30 [ 5560.047036] find_inode_fast+0x6d/0xc0 [ 5560.048473] iget_locked+0x79/0x230 [ 5560.049933] f2fs_iget+0x27/0x1200 [f2fs] [ 5560.051496] f2fs_lookup+0x18c/0x3e0

Re: [f2fs-dev] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

2022-02-28 Thread David Laight
From: Matthew Wilcox > Sent: 28 February 2022 20:16 > > On Mon, Feb 28, 2022 at 12:10:24PM -0800, Linus Torvalds wrote: > > We can do > > > > typeof(pos) pos > > > > in the 'for ()' loop, and never use __iter at all. > > > > That means that inside the for-loop, we use a _different_ 'pos'

Re: [f2fs-dev] [PATCH 2/2] f2fs: pass the bio operation to bio_alloc_bioset

2022-02-28 Thread Chao Yu
On 2022/2/28 20:41, Christoph Hellwig wrote: Refactor block I/O code so that the bio operation and known flags are set at bio allocation time. Only the later updated flags are updated on the fly. Signed-off-by: Christoph Hellwig Reviewed-by: Chao Yu Thanks,

Re: [f2fs-dev] [PATCH 1/2] f2fs: don't pass a bio to f2fs_target_device

2022-02-28 Thread Chao Yu
On 2022/2/28 20:41, Christoph Hellwig wrote: Set the bdev at bio allocation time by changing the f2fs_target_device calling conventions, so that no bio needs to be passed in. Signed-off-by: Christoph Hellwig Reviewed-by: Chao Yu Thanks, ___

Re: [f2fs-dev] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

2022-02-28 Thread David Laight
From: Linus Torvalds > Sent: 28 February 2022 19:56 > On Mon, Feb 28, 2022 at 4:19 AM Christian König > wrote: > > > > I don't think that using the extra variable makes the code in any way > > more reliable or easier to read. > > So I think the next step is to do the attached patch (which

Re: [f2fs-dev] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

2022-02-28 Thread Linus Torvalds
On Mon, Feb 28, 2022 at 4:45 PM Linus Torvalds wrote: > > Yeah, except that's ugly beyond belief, plus it's literally not what > we do in the kernel. (Of course, I probably shouldn't have used 'min()' as an example, because that is actually one of the few places where we do exactly that, using

Re: [f2fs-dev] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

2022-02-28 Thread Linus Torvalds
On Mon, Feb 28, 2022 at 4:38 PM Segher Boessenkool wrote: > > In C its scope is the rest of the declaration and the entire loop, not > anything after it. This was the same in C++98 already, btw (but in > pre-standard versions of C++ things were like you remember, yes, and it > was painful).

Re: [f2fs-dev] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

2022-02-28 Thread Linus Torvalds
On Mon, Feb 28, 2022 at 3:26 PM Matthew Wilcox wrote: > > #define ___PASTE(a, b) a##b > #define __PASTE(a, b) ___PASTE(a, b) > #define _min(a, b, u) ({ \ Yeah, except that's ugly beyond belief, plus it's literally not what we do in the kernel. Really. The "-Wshadow doesn't work on the

Re: [f2fs-dev] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

2022-02-28 Thread Segher Boessenkool
On Mon, Feb 28, 2022 at 05:28:58PM -0500, James Bottomley wrote: > On Mon, 2022-02-28 at 23:59 +0200, Mike Rapoport wrote: > > > > On February 28, 2022 10:42:53 PM GMT+02:00, James Bottomley < > > james.bottom...@hansenpartnership.com> wrote: > > > On Mon, 2022-02-28 at 21:07 +0100, Christian

Re: [f2fs-dev] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

2022-02-28 Thread Linus Torvalds
On Mon, Feb 28, 2022 at 1:47 PM Jakob Koschel wrote: > > The goal of this is to get compiler warnings right? This would indeed be > great. Yes, so I don't mind having a one-time patch that has been gathered using some automated checker tool, but I don't think that works from a long-term

Re: [f2fs-dev] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

2022-02-28 Thread Matthew Wilcox
On Mon, Feb 28, 2022 at 12:37:15PM -0800, Linus Torvalds wrote: > On Mon, Feb 28, 2022 at 12:16 PM Matthew Wilcox wrote: > > > > Then we can never use -Wshadow ;-( I'd love to be able to turn it on; > > it catches real bugs. > > Oh, we already can never use -Wshadow regardless of things like

Re: [f2fs-dev] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

2022-02-28 Thread Barnabás Pőcze via Linux-f2fs-devel
Hi 2022. február 28., hétfő 23:28 keltezéssel, James Bottomley írta: > [...] > Well, yes, but my objection is more to the size of churn than the > desire to do loop local. I'm not even sure loop local is possible, > because it's always annoyed me that for (int i = 0; ... in C++ defines > i in

Re: [f2fs-dev] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

2022-02-28 Thread James Bottomley
On Mon, 2022-02-28 at 23:59 +0200, Mike Rapoport wrote: > > On February 28, 2022 10:42:53 PM GMT+02:00, James Bottomley < > james.bottom...@hansenpartnership.com> wrote: > > On Mon, 2022-02-28 at 21:07 +0100, Christian König wrote: [...] > > > > I do wish we could actually poison the 'pos' value

Re: [f2fs-dev] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

2022-02-28 Thread Jakob Koschel
> On 28. Feb 2022, at 21:56, Christian König wrote: > > > > Am 28.02.22 um 21:42 schrieb James Bottomley: >> On Mon, 2022-02-28 at 21:07 +0100, Christian König wrote: >>> Am 28.02.22 um 20:56 schrieb Linus Torvalds: On Mon, Feb 28, 2022 at 4:19 AM Christian König wrote:

Re: [f2fs-dev] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

2022-02-28 Thread Segher Boessenkool
On Mon, Feb 28, 2022 at 12:14:44PM -0800, Linus Torvalds wrote: > On Mon, Feb 28, 2022 at 12:10 PM Linus Torvalds > wrote: > > > > We can do > > > > typeof(pos) pos > > > > in the 'for ()' loop, and never use __iter at all. > > > > That means that inside the for-loop, we use a _different_

Re: [f2fs-dev] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

2022-02-28 Thread Mike Rapoport
On February 28, 2022 10:42:53 PM GMT+02:00, James Bottomley wrote: >On Mon, 2022-02-28 at 21:07 +0100, Christian König wrote: >> Am 28.02.22 um 20:56 schrieb Linus Torvalds: >> > On Mon, Feb 28, 2022 at 4:19 AM Christian König >> > wrote: >> > > I don't think that using the extra variable

Re: [f2fs-dev] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

2022-02-28 Thread Jakob Koschel
> On 28. Feb 2022, at 21:10, Linus Torvalds > wrote: > > On Mon, Feb 28, 2022 at 12:03 PM Linus Torvalds > wrote: >> >> Side note: we do need *some* way to do it. > > Ooh. > > This patch is a work of art. > > And I mean that in the worst possible way. > > We can do > >

Re: [f2fs-dev] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

2022-02-28 Thread Christian König via Linux-f2fs-devel
Am 28.02.22 um 20:56 schrieb Linus Torvalds: On Mon, Feb 28, 2022 at 4:19 AM Christian König wrote: I don't think that using the extra variable makes the code in any way more reliable or easier to read. So I think the next step is to do the attached patch (which requires that "-std=gnu11"

Re: [f2fs-dev] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

2022-02-28 Thread Christian König via Linux-f2fs-devel
Am 28.02.22 um 21:42 schrieb James Bottomley: On Mon, 2022-02-28 at 21:07 +0100, Christian König wrote: Am 28.02.22 um 20:56 schrieb Linus Torvalds: On Mon, Feb 28, 2022 at 4:19 AM Christian König wrote: [SNIP] Anybody have any ideas? I think we should look at the use cases why code is

Re: [f2fs-dev] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

2022-02-28 Thread Jeffrey Walton
On Mon, Feb 28, 2022 at 3:45 PM James Bottomley wrote: > ... > > Just from skimming over the patches to change this and experience > > with the drivers/subsystems I help to maintain I think the primary > > pattern looks something like this: > > > > list_for_each_entry(entry, head, member) { > >

Re: [f2fs-dev] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

2022-02-28 Thread Johannes Berg
On Mon, 2022-02-28 at 20:16 +, Matthew Wilcox wrote: > On Mon, Feb 28, 2022 at 12:10:24PM -0800, Linus Torvalds wrote: > > We can do > > > > typeof(pos) pos > > > > in the 'for ()' loop, and never use __iter at all. > > > > That means that inside the for-loop, we use a _different_

Re: [f2fs-dev] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

2022-02-28 Thread James Bottomley
On Mon, 2022-02-28 at 21:56 +0100, Christian König wrote: > > Am 28.02.22 um 21:42 schrieb James Bottomley: > > On Mon, 2022-02-28 at 21:07 +0100, Christian König wrote: > > > Am 28.02.22 um 20:56 schrieb Linus Torvalds: > > > > On Mon, Feb 28, 2022 at 4:19 AM Christian König > > > > wrote: > >

Re: [f2fs-dev] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

2022-02-28 Thread James Bottomley
On Mon, 2022-02-28 at 21:07 +0100, Christian König wrote: > Am 28.02.22 um 20:56 schrieb Linus Torvalds: > > On Mon, Feb 28, 2022 at 4:19 AM Christian König > > wrote: > > > I don't think that using the extra variable makes the code in any > > > way > > > more reliable or easier to read. > > So I

Re: [f2fs-dev] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

2022-02-28 Thread Linus Torvalds
On Mon, Feb 28, 2022 at 12:16 PM Matthew Wilcox wrote: > > Then we can never use -Wshadow ;-( I'd love to be able to turn it on; > it catches real bugs. Oh, we already can never use -Wshadow regardless of things like this. That bridge hasn't just been burned, it never existed in the first

Re: [f2fs-dev] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

2022-02-28 Thread Linus Torvalds
On Mon, Feb 28, 2022 at 12:29 PM Johannes Berg wrote: > > If we're willing to change the API for the macro, we could do > > list_for_each_entry(type, pos, head, member) > > and then actually take advantage of -Wshadow? See my reply to Willy. There is no way -Wshadow will ever happen. I

Re: [f2fs-dev] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

2022-02-28 Thread Linus Torvalds
On Mon, Feb 28, 2022 at 12:10 PM Linus Torvalds wrote: > > We can do > > typeof(pos) pos > > in the 'for ()' loop, and never use __iter at all. > > That means that inside the for-loop, we use a _different_ 'pos' than outside. The thing that makes me throw up in my mouth a bit is that in

Re: [f2fs-dev] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

2022-02-28 Thread Matthew Wilcox
On Mon, Feb 28, 2022 at 12:10:24PM -0800, Linus Torvalds wrote: > We can do > > typeof(pos) pos > > in the 'for ()' loop, and never use __iter at all. > > That means that inside the for-loop, we use a _different_ 'pos' than outside. Then we can never use -Wshadow ;-( I'd love to be

Re: [f2fs-dev] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

2022-02-28 Thread Linus Torvalds
On Mon, Feb 28, 2022 at 11:56 AM Linus Torvalds wrote: > > I do wish we could actually poison the 'pos' value after the loop > somehow - but clearly the "might be uninitialized" I was hoping for > isn't the way to do it. Side note: we do need *some* way to do it. Because if we make that change,

Re: [f2fs-dev] [PATCH 1/6] drivers: usb: remove usage of list iterator past the loop body

2022-02-28 Thread Joe Perches
On Mon, 2022-02-28 at 14:24 +0300, Dan Carpenter wrote: > a multi-line indent gets curly braces for readability even though > it's not required by C. And then both sides would get curly braces. That's more your personal preference than a coding style guideline.

Re: [f2fs-dev] [PATCH 1/6] drivers: usb: remove usage of list iterator past the loop body

2022-02-28 Thread Dan Carpenter
On Mon, Feb 28, 2022 at 01:03:36PM +0100, Jakob Koschel wrote: > >> @@ -954,7 +957,6 @@ net2272_dequeue(struct usb_ep *_ep, struct usb_request > >> *_req) > >>dev_dbg(ep->dev->dev, "unlink (%s) pio\n", _ep->name); > >>net2272_done(ep, req, -ECONNRESET); > >>} > >> -

Re: [f2fs-dev] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

2022-02-28 Thread Dan Carpenter
On Mon, Feb 28, 2022 at 12:08:18PM +0100, Jakob Koschel wrote: > diff --git a/drivers/scsi/scsi_transport_sas.c > b/drivers/scsi/scsi_transport_sas.c > index 4ee578b181da..a8cbd90db9d2 100644 > --- a/drivers/scsi/scsi_transport_sas.c > +++ b/drivers/scsi/scsi_transport_sas.c > @@ -1060,26

Re: [f2fs-dev] [PATCH 6/6] treewide: remove check of list iterator against head past the loop body

2022-02-28 Thread Dan Carpenter
On Mon, Feb 28, 2022 at 12:08:22PM +0100, Jakob Koschel wrote: > diff --git a/drivers/infiniband/hw/hfi1/tid_rdma.c > b/drivers/infiniband/hw/hfi1/tid_rdma.c > index 2a7abf7a1f7f..a069847b56aa 100644 > --- a/drivers/infiniband/hw/hfi1/tid_rdma.c > +++ b/drivers/infiniband/hw/hfi1/tid_rdma.c > @@

Re: [f2fs-dev] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

2022-02-28 Thread Christian König via Linux-f2fs-devel
Am 28.02.22 um 12:08 schrieb Jakob Koschel: If the list does not contain the expected element, the value of list_for_each_entry() iterator will not point to a valid structure. To avoid type confusion in such case, the list iterator scope will be limited to list_for_each_entry() loop. We

[f2fs-dev] [PATCH 2/2] f2fs: pass the bio operation to bio_alloc_bioset

2022-02-28 Thread Christoph Hellwig
Refactor block I/O code so that the bio operation and known flags are set at bio allocation time. Only the later updated flags are updated on the fly. Signed-off-by: Christoph Hellwig --- fs/f2fs/data.c | 70 +- 1 file changed, 29 insertions(+),

[f2fs-dev] fully convert f2fs to the new bio allocation interface

2022-02-28 Thread Christoph Hellwig
Hi Jaegeuk and Chao, as of the for-5.18/block tree, the bio allocation interfaces expect the block_device and operation passed. This fixes f2fs up to match that scheme. It could go into Jens' alloc-cleanups branch if there are no conflicts in the f2fs tree.

[f2fs-dev] [PATCH 1/2] f2fs: don't pass a bio to f2fs_target_device

2022-02-28 Thread Christoph Hellwig
Set the bdev at bio allocation time by changing the f2fs_target_device calling conventions, so that no bio needs to be passed in. Signed-off-by: Christoph Hellwig --- fs/f2fs/data.c | 25 + fs/f2fs/f2fs.h | 2 +- 2 files changed, 14 insertions(+), 13 deletions(-) diff

Re: [f2fs-dev] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

2022-02-28 Thread Jakob Koschel
> On 28. Feb 2022, at 12:20, Greg KH wrote: > > On Mon, Feb 28, 2022 at 12:08:18PM +0100, Jakob Koschel wrote: >> If the list does not contain the expected element, the value of >> list_for_each_entry() iterator will not point to a valid structure. >> To avoid type confusion in such case, the

Re: [f2fs-dev] [PATCH 1/6] drivers: usb: remove usage of list iterator past the loop body

2022-02-28 Thread Jakob Koschel
> On 28. Feb 2022, at 12:24, Dan Carpenter wrote: > > On Mon, Feb 28, 2022 at 12:08:17PM +0100, Jakob Koschel wrote: >> diff --git a/drivers/usb/gadget/udc/at91_udc.c >> b/drivers/usb/gadget/udc/at91_udc.c >> index 9040a0561466..0fd0307bc07b 100644 >> --- a/drivers/usb/gadget/udc/at91_udc.c

Re: [f2fs-dev] [PATCH 3/6] treewide: fix incorrect use to determine if list is empty

2022-02-28 Thread Dan Carpenter
On Mon, Feb 28, 2022 at 12:08:19PM +0100, Jakob Koschel wrote: > The list iterator value will *always* be set by list_for_each_entry(). > It is incorrect to assume that the iterator value will be NULL if the > list is empty. > > Instead of checking the pointer it should be checked if > the list

Re: [f2fs-dev] [PATCH 1/6] drivers: usb: remove usage of list iterator past the loop body

2022-02-28 Thread Dan Carpenter
On Mon, Feb 28, 2022 at 12:08:17PM +0100, Jakob Koschel wrote: > diff --git a/drivers/usb/gadget/udc/at91_udc.c > b/drivers/usb/gadget/udc/at91_udc.c > index 9040a0561466..0fd0307bc07b 100644 > --- a/drivers/usb/gadget/udc/at91_udc.c > +++ b/drivers/usb/gadget/udc/at91_udc.c > @@ -150,13 +150,14

Re: [f2fs-dev] [PATCH 6/6] treewide: remove check of list iterator against head past the loop body

2022-02-28 Thread Dominique Martinet
This is a bit more work (and a lot more noise), but I'd prefer if this were split into as many patches as there are components. I'm not going to review the parts of the patches that don't concern me, and if something turns out to be a problem later one (it shouldn't but one never knows) it'll be

Re: [f2fs-dev] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

2022-02-28 Thread Greg KH
On Mon, Feb 28, 2022 at 12:08:18PM +0100, Jakob Koschel wrote: > If the list does not contain the expected element, the value of > list_for_each_entry() iterator will not point to a valid structure. > To avoid type confusion in such case, the list iterator > scope will be limited to

[f2fs-dev] [PATCH 6/6] treewide: remove check of list iterator against head past the loop body

2022-02-28 Thread Jakob Koschel
When list_for_each_entry() completes the iteration over the whole list without breaking the loop, the iterator value will be a bogus pointer computed based on the head element. While it is safe to use the pointer to determine if it was computed based on the head element, either with

[f2fs-dev] [PATCH 5/6] treewide: remove dereference of list iterator after loop body

2022-02-28 Thread Jakob Koschel
The list iterator variable will be a bogus pointer if no break was hit. Dereferencing it could load *any* out-of-bounds/undefined value making it unsafe to use that in the comparision to determine if the specific element was found. This is fixed by using a separate list iterator variable for the

[f2fs-dev] [PATCH 0/6] Remove usage of list iterator past the loop body

2022-02-28 Thread Jakob Koschel
This is the first patch removing several categories of use cases of the list iterator variable past the loop. This is follow up to the discussion in: https://lore.kernel.org/all/20220217184829.1991035-1-jakobkosc...@gmail.com/ As concluded in:

[f2fs-dev] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr

2022-02-28 Thread Jakob Koschel
If the list does not contain the expected element, the value of list_for_each_entry() iterator will not point to a valid structure. To avoid type confusion in such case, the list iterator scope will be limited to list_for_each_entry() loop. In preparation to limiting scope of a list iterator to

[f2fs-dev] [PATCH 1/6] drivers: usb: remove usage of list iterator past the loop body

2022-02-28 Thread Jakob Koschel
If the list representing the request queue does not contain the expected request, the value of list_for_each_entry() iterator will not point to a valid structure. To avoid type confusion in such case, the list iterator scope will be limited to list_for_each_entry() loop. In preparation to

[f2fs-dev] [PATCH 4/6] drivers: remove unnecessary use of list iterator variable

2022-02-28 Thread Jakob Koschel
When list_for_each_entry() completes the iteration over the whole list without breaking the loop, the iterator value will *always* be a bogus pointer computed based on the head element. To avoid type confusion use the actual list head directly instead of last iterator value. Signed-off-by: Jakob

[f2fs-dev] [PATCH 3/6] treewide: fix incorrect use to determine if list is empty

2022-02-28 Thread Jakob Koschel
The list iterator value will *always* be set by list_for_each_entry(). It is incorrect to assume that the iterator value will be NULL if the list is empty. Instead of checking the pointer it should be checked if the list is empty. In acpi_get_pmu_hw_inf() instead of setting the pointer to NULL on