Re: [PATCH net-next 00/11] net: sched: cls_u32 Various improvements

On Sun, Oct 07, 2018 at 10:55:52PM -0700, David Miller wrote: > From: Al Viro > Date: Mon, 8 Oct 2018 06:45:15 +0100 > > > Er... Both are due to missing in the very beginning of the series (well, on > > top of "net: sched: cls_u32: fix hnode refcounting") co

Re: [PATCH net-next 00/11] net: sched: cls_u32 Various improvements

sing in the very beginning of the series (well, on top of "net: sched: cls_u32: fix hnode refcounting") commit Author: Al Viro Date: Mon Sep 3 14:39:02 2018 -0400 net: sched: cls_u32: mark root hnode explicitly ... and produce consistent error on attempt to delete such. E

Re: [PATCH] socket: fix struct ifreq size in compat ioctl

On Thu, Sep 13, 2018 at 01:49:09PM +0200, Johannes Berg wrote: > From: Johannes Berg > > As reported by Reobert O'Callahan, since Viro's commit to kill > dev_ifsioc() we attempt to copy too much data in compat mode, > which may lead to EFAULT when the 32-bit version of struct ifreq > sits

Re: [PATCH net 00/13] cls_u32 cleanups and fixes.

On Sun, Sep 09, 2018 at 03:15:38PM +0100, Al Viro wrote: > Umm... Interesting - TCA_U32_SEL is not the only thing that > gets ignored there; TCA_U32_MARK gets the same treatment. > And then there's a lovely question what to do with n->pf - > it's an array of n->sel.nkeys counte

Re: [PATCH net 00/13] cls_u32 cleanups and fixes.

On Sun, Sep 09, 2018 at 08:58:50AM -0400, Jamal Hadi Salim wrote: > > Since you have the momentum here: i noticed something > unusual while i was trying to craft a test that would > vet some of your changes. This has nothing to do with > your changes, same happens on my stock debian laptop > with

[PATCH net 13/13] net: sched: cls_u32: simplify the hell out u32_delete() emptiness check

From: Al Viro Now that we have the knode count, we can instantly check if any hnodes are non-empty. And that kills the check for extra references to root hnode - those could happen only if there was a knode to carry such a link. Signed-off-by: Al Viro --- net/sched/cls_u32.c | 48

[PATCH net 12/13] net: sched: cls_u32: keep track of knodes count in tc_u_common

From: Al Viro allows to simplify u32_delete() considerably Signed-off-by: Al Viro --- net/sched/cls_u32.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c index f6bb3885598d..86cbe4f5800e 100644 --- a/net/sched/cls_u32.c +++ b/net/sched

[PATCH net 11/13] net: sched: cls_u32: get rid of hnode ->tp_c and tp_c argument of u32_set_parms()

From: Al Viro the latter is redundant, the former - never read... Signed-off-by: Al Viro --- net/sched/cls_u32.c | 12 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c index 12757e3ec8d8..f6bb3885598d 100644 --- a/net/sched

[PATCH net 09/13] net: sched: cls_u32: pass tc_u_common to u32_set_parms() instead of tc_u_hnode

From: Al Viro the only thing we used ht for was ht->tp_c and callers can get that without going through ->tp_c at all; start with lifting that into the callers, next commits will massage those, eventually removing ->tp_c altogether. Signed-off-by: Al Viro --- net/sched/cls_

[PATCH net 10/13] net: sched: cls_u32: the tp_c argument of u32_set_parms() is always tp->data

From: Al Viro It must be tc_u_common associated with that tp (i.e. tp->data). Proof: * both ->ht_up and ->tp_c are assign-once * ->tp_c of anything inserted into tp_c->hlist is tp_c * hnodes never get reinserted into the lists or moved between those, so

[PATCH net 08/13] net: sched: cls_u32: clean tc_u_common hashtable

From: Al Viro * calculate key *once*, not for each hash chain element * let tc_u_hash() return the pointer to chain head rather than index - callers are cleaner that way. Signed-off-by: Al Viro --- net/sched/cls_u32.c | 24 +--- 1 file changed, 9 insertions(+), 15

[PATCH net 07/13] net: sched: cls_u32: get rid of tc_u_common ->rcu

From: Al Viro unused Signed-off-by: Al Viro --- net/sched/cls_u32.c | 1 - 1 file changed, 1 deletion(-) diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c index 281ac954511c..1bfbfcab7260 100644 --- a/net/sched/cls_u32.c +++ b/net/sched/cls_u32.c @@ -98,7 +98,6 @@ struct tc_u_common

[PATCH net 06/13] net: sched: cls_u32: get rid of tc_u_knode ->tp

From: Al Viro not used anymore Signed-off-by: Al Viro --- net/sched/cls_u32.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c index d11862823911..281ac954511c 100644 --- a/net/sched/cls_u32.c +++ b/net/sched/cls_u32.c @@ -68,7 +68,6 @@ struct

[PATCH net 04/13] net: sched: cls_u32: make sure that divisor is a power of 2

From: Al Viro Signed-off-by: Al Viro --- net/sched/cls_u32.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c index 72459b09d910..d9923d474b65 100644 --- a/net/sched/cls_u32.c +++ b/net/sched/cls_u32.c @@ -994,7 +994,11

[PATCH net 05/13] net: sched: cls_u32: get rid of unused argument of u32_destroy_key()

From: Al Viro Signed-off-by: Al Viro --- net/sched/cls_u32.c | 13 ++--- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c index d9923d474b65..d11862823911 100644 --- a/net/sched/cls_u32.c +++ b/net/sched/cls_u32.c @@ -406,8 +406,7

[PATCH net 03/13] net: sched: cls_u32: disallow linking to root hnode

From: Al Viro Operation makes no sense. Nothing will actually break if we do so (depth limit in u32_classify() will prevent infinite loops), but according to maintainers it's best prohibited outright. NOTE: doing so guarantees that u32_destroy() will trigger the call of u32_destroy_hnode(); we

[PATCH net 02/13] net: sched: cls_u32: mark root hnode explicitly

From: Al Viro ... and produce consistent error on attempt to delete such. Existing check in u32_delete() is inconsistent - after tc qdisc add dev eth0 ingress tc filter add dev eth0 parent : protocol ip prio 100 handle 1: u32 divisor 1 tc filter add dev eth0 parent : protocol ip prio

[PATCH net 00/13] cls_u32 cleanups and fixes.

From: Al Viro A series of net/sched/cls_u32.c cleanups and fixes: 1) fix hnode refcounting. Refcounting for tc_u_hnode is broken; it's not hard to trigger oopsen (including one inside an interrupt handler, with resulting panic) as well as memory corruption. Definitely -stable fodder

[PATCH net 01/13] net: sched: cls_u32: fix hnode refcounting

From: Al Viro cls_u32.c misuses refcounts for struct tc_u_hnode - it counts references via ->hlist and via ->tp_root together. u32_destroy() drops the former and, in case when there had been links, leaves the sucker on the list. As the result, there's nothing to protect it from getting

Re: [PATCH 1/7] fix hnode refcounting

On Fri, Sep 07, 2018 at 08:13:56AM -0400, Jamal Hadi Salim wrote: > > } else { > > bool last; > > > > err = tfilter_del_notify(net, skb, n, tp, block, > > q, parent, fh, false, , > >

Re: [PATCH 6/7] get rid of tc_u_common ->rcu

On Thu, Sep 06, 2018 at 09:18:47PM -0700, Cong Wang wrote: > On Wed, Sep 5, 2018 at 12:04 PM Al Viro wrote: > > > > From: Al Viro > > > > unused > > > > Signed-off-by: Al Viro > > --- > > net/sched/cls_u32.c | 1 - > > 1 file changed,

Re: [PATCH 2/7] mark root hnode explicitly

On Thu, Sep 06, 2018 at 08:23:36PM -0700, Cong Wang wrote: > Pretty sure there is a 'tp' in u32_set_parms() parameter list. > > Are you saying it is not what you want? If so, why? > > More importantly, why this information is again missing in your > changelog? This patch is definitely not

Re: [PATCH 2/7] mark root hnode explicitly

On Thu, Sep 06, 2018 at 07:57:25PM -0700, Cong Wang wrote: > > - if (root_ht == ht) { > > + if (ht->is_root) { > > > What's wrong with comparing pointers with root ht? The fact that there may be more than one tcf_proto sharing tp->data. > >

Re: [PATCH 1/7] fix hnode refcounting

On Thu, Sep 06, 2018 at 06:21:09AM -0400, Jamal Hadi Salim wrote: > For networking patches, subject should be reflective of tree and > subsystem. Example for this one: > "[PATCH net 1/7]:net: sched: cls_u32: fix hnode refcounting" > Also useful to have a cover letter summarizing the patchset > in

Re: [PATCH 2/7] mark root hnode explicitly

On Thu, Sep 06, 2018 at 06:34:00AM -0400, Jamal Hadi Salim wrote: > On 2018-09-06 6:28 a.m., Jamal Hadi Salim wrote: > > On 2018-09-05 3:04 p.m., Al Viro wrote: > > > From: Al Viro > > > > > > ... and disallow deleting or linking to such > > > > &

[PATCH 1/7] fix hnode refcounting

From: Al Viro cls_u32.c misuses refcounts for struct tc_u_hnode - it counts references via ->hlist and via ->tp_root together. u32_destroy() drops the former and, in case when there had been links, leaves the sucker on the list. As the result, there's nothing to protect it from getting

[PATCH 4/7] get rid of unused argument of u32_destroy_key()

From: Al Viro Signed-off-by: Al Viro --- net/sched/cls_u32.c | 13 ++--- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c index 5816288810cc..3311aacad6c3 100644 --- a/net/sched/cls_u32.c +++ b/net/sched/cls_u32.c @@ -406,8 +406,7

[PATCH 3/7] make sure that divisor is a power of 2

From: Al Viro Signed-off-by: Al Viro --- net/sched/cls_u32.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c index 9ea5f2be907b..5816288810cc 100644 --- a/net/sched/cls_u32.c +++ b/net/sched/cls_u32.c @@ -995,7 +995,11

[PATCHES] cls_u32 cleanups and fixes

Several cls_u32 patches: fixing refcounting, preventing links to and deletion of root hnodes, validating divisor. Plus cleanups - removal of some useless fields and saner handling of tc_u_common hashtable. The first 3 in series are fixes (and -stable fodder), the rest - cleanups. Branch

[PATCH 5/7] get rid of tc_u_knode ->tp

From: Al Viro not used anymore Signed-off-by: Al Viro --- net/sched/cls_u32.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c index 3311aacad6c3..8a1a573487bd 100644 --- a/net/sched/cls_u32.c +++ b/net/sched/cls_u32.c @@ -68,7 +68,6 @@ struct

[PATCH 2/7] mark root hnode explicitly

From: Al Viro ... and disallow deleting or linking to such Signed-off-by: Al Viro --- net/sched/cls_u32.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c index 3f985f29ef30..9ea5f2be907b 100644 --- a/net/sched

[PATCH 7/7] clean tc_u_common hashtable

From: Al Viro * calculate key *once*, not for each hash chain element * let tc_u_hash() return the pointer to chain head rather than index - callers are cleaner that way. Signed-off-by: Al Viro --- net/sched/cls_u32.c | 22 -- 1 file changed, 8 insertions(+), 14 deletions

[PATCH 6/7] get rid of tc_u_common ->rcu

From: Al Viro unused Signed-off-by: Al Viro --- net/sched/cls_u32.c | 1 - 1 file changed, 1 deletion(-) diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c index 8a1a573487bd..be9240ae1417 100644 --- a/net/sched/cls_u32.c +++ b/net/sched/cls_u32.c @@ -98,7 +98,6 @@ struct tc_u_common

Re: [PATCH] net: sched: Fix memory exposure from short TCA_U32_SEL

On Sun, Aug 26, 2018 at 11:35:17PM -0400, Julia Lawall wrote: > * x = \(kmalloc\|kzalloc\|devm_kmalloc\|devm_kzalloc\)(...) I can name several you've missed right off the top of my head - vmalloc, kvmalloc, kmem_cache_alloc, kmem_cache_zalloc, variants with _trace slapped on, and that is not to

Re: [PATCH] net: sched: Fix memory exposure from short TCA_U32_SEL

On Sun, Aug 26, 2018 at 10:00:46PM -0400, Julia Lawall wrote: > > > On Sun, 26 Aug 2018, Al Viro wrote: > > > On Sun, Aug 26, 2018 at 03:26:54PM -0700, Joe Perches wrote: > > > On Sun, 2018-08-26 at 22:24 +0100, Al Viro wrote: > > > > On Sun, Aug 26, 2018

Re: [endianness bug] cxgb4: mk_act_open_req() buggers ->{local,peer}_ip on big-endian hosts

On Fri, Aug 17, 2018 at 07:59:44PM +0100, Al Viro wrote: > On Fri, Aug 17, 2018 at 07:58:41PM +0100, Al Viro wrote: > > On Fri, Aug 17, 2018 at 07:09:49PM +0100, Al Viro wrote: > > > > > Re that code - are you sure it doesn't need le64_to_cpu(*src)? Because > >

Re: [endianness bug] cxgb4: mk_act_open_req() buggers ->{local,peer}_ip on big-endian hosts

On Fri, Aug 17, 2018 at 07:58:41PM +0100, Al Viro wrote: > On Fri, Aug 17, 2018 at 07:09:49PM +0100, Al Viro wrote: > > > Re that code - are you sure it doesn't need le64_to_cpu(*src)? Because > > from what > > I understand about PCI (which matches just fine to

Re: [endianness bug] cxgb4: mk_act_open_req() buggers ->{local,peer}_ip on big-endian hosts

On Fri, Aug 17, 2018 at 07:09:49PM +0100, Al Viro wrote: > Re that code - are you sure it doesn't need le64_to_cpu(*src)? Because from > what > I understand about PCI (which matches just fine to the comments in the same > driver), > you probably do need that. Again, the only r

Re: [endianness bug] cxgb4: mk_act_open_req() buggers ->{local,peer}_ip on big-endian hosts

On Fri, Aug 17, 2018 at 04:43:20PM +0100, Al Viro wrote: > On Fri, Aug 17, 2018 at 06:35:41PM +0530, Ganesh Goudar wrote: > > Thanks, Al. The patch looks good to me but it does not seem > > to be showing up in patchwork, should I resend the patch on > > your behalf to net

Re: [endianness bug] cxgb4: mk_act_open_req() buggers ->{local,peer}_ip on big-endian hosts

On Fri, Aug 17, 2018 at 06:35:41PM +0530, Ganesh Goudar wrote: > Thanks, Al. The patch looks good to me but it does not seem > to be showing up in patchwork, should I resend the patch on > your behalf to net tree ? Umm... I thought net-next had been closed until -rc1, hadn't it? Anyway,

[RFC][bug?] "net/act_pedit: Introduce 'add' operation" is broken for anything wider than an octet

The code doing addition in that commit is + switch (cmd) { + case TCA_PEDIT_KEY_EX_CMD_SET: + val = tkey->val; + break; + case TCA_PEDIT_KEY_EX_CMD_ADD: +

Re: [endianness bug] cxgb4: mk_act_open_req() buggers ->{local,peer}_ip on big-endian hosts

oiding both the b-e problems and getting rid of a lot of LoC, including an unpleasant macro. Signed-off-by: Al Viro --- diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c index 3db969eefba9..020ca0121fb4 100644 --- a/drivers/net/etherne

Re: [endianness bug?] cxgb4_next_header .match_val/.match_mask should be net-endian

rong git tree, please drop us a note to > help improve the system] > > url: > https://github.com/0day-ci/linux/commits/Al-Viro/cxgb4_next_header-match_val-match_mask-should-be-net-endian/20180806-183127 > reproduce: > # apt-get install sparse > make ARCH

[RFC] weirdness in cxgb3_main.c:init_tp_parity()

for (i = 0; i < 2048; i++) { ... req->l2t_idx = htonl(V_L2T_W_IDX(i)); ... in there is very odd; l2t_idx is a 16bit field, and #define V_L2T_W_IDX(x) ((x) << S_L2T_W_IDX) #define S_L2T_W_IDX0 IOW, we are taking htonl(something in range 0..2047) and shove it

[endianness bug] cxgb4: mk_act_open_req() buggers ->{local,peer}_ip on big-endian hosts

As far as I know, T4 PCIe cards do exist, so it's not as if that thing could only be found on little-endian systems... Signed-off-by: Al Viro --- diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_filter.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_filter.c index 00fc5f1afb1d..7dddb9e748b8

Re: [endianness bug?] cxgb4_next_header .match_val/.match_mask should be net-endian

On Sun, Aug 05, 2018 at 04:28:11PM +0100, Al Viro wrote: > On little-endian host those do yield the right values - e.g. 0x1100 is > {0, 17, 0, 0}, etc. On big-endian, though, these will end up checking > in IPv4 case the octet at offset 10 (i.e. upper 16 bits of checksum) and fo

[endianness bug?] cxgb4_next_header .match_val/.match_mask should be net-endian

the right thing both on l-e and b-e. Comments? Signed-off-by: Al Viro --- diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32_parse.h b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32_parse.h index a4b99edcc339..ec226b1cebf4 100644 --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_u32_pa

[PATCH][mwifiex] fix braino in mwifiex_fw_dump_info_event()

... and on little-endian host it even worked ;-) Signed-off-by: Al Viro --- diff --git a/drivers/net/wireless/marvell/mwifiex/sta_event.c b/drivers/net/wireless/marvell/mwifiex/sta_event.c index 03a6492662ca..1cd42a6a25c9 100644 --- a/drivers/net/wireless/marvell/mwifiex/sta_event.c

[PATCH] mellanox: fix the dport endianness in call of __inet6_lookup_established()

__inet6_lookup_established() expect th->dport passed in host-endian, not net-endian. The reason is microoptimization in __inet6_lookup(), but if you use the lower-level helpers, you have to play by their rules... Signed-off-by: Al Viro --- diff --git a/drivers/net/ethernet/mellanox/m

[stmmac][bug?] endianness of Flexible RX Parser code

The values passed in struct tc_u32_sel ->mask and ->val are 32bit net-endian. Your tc_fill_entry() does this: data = sel->keys[0].val; mask = sel->keys[0].mask; ... entry->frag_ptr = frag; entry->val.match_en = (mask << (rem * 8)) &

Re: [PATCH net-next] net/tls: Do not call msg_data_left() twice

On Tue, Jul 24, 2018 at 04:41:18PM +0530, Vakul Garg wrote: > In function tls_sw_sendmsg(), msg_data_left() needs to be called only > once. The second invocation of msg_data_left() for assigning variable > try_to_copy can be removed and merged with the first one. You do realize that

Re: [PATCH 6/6] fs: replace f_ops->get_poll_head with a static ->f_poll_head pointer

On Thu, Jun 28, 2018 at 03:55:35PM -0700, Linus Torvalds wrote: > > You are misreading that mess. What he's trying to do (other than surviving > > the awful clusterfuck around cancels) is to handle the decision what to > > report to userland right in the wakeup callback. *That* is what really >

Re: [PATCH 6/6] fs: replace f_ops->get_poll_head with a static ->f_poll_head pointer

On Thu, Jun 28, 2018 at 03:35:03PM -0700, Linus Torvalds wrote: > Yes, the AIO poll implementation did it under the spinlock. > > But there's no good *reason* for that. The "aio_poll()" function > itself is called in perfectly fine blocking context. aio_poll() is not a problem. It's wakeup

Re: [PATCH 6/6] fs: replace f_ops->get_poll_head with a static ->f_poll_head pointer

On Thu, Jun 28, 2018 at 10:30:27PM +0100, Al Viro wrote: > I'm not saying that blocking on other things is a bug; some of such *are* > bogus, > but a lot aren't really broken. What I said is that in a lot of cases we > really > have hard "no blocking other than in callback

Re: [PATCH 6/6] fs: replace f_ops->get_poll_head with a static ->f_poll_head pointer

On Thu, Jun 28, 2018 at 02:11:17PM -0700, Linus Torvalds wrote: > On Thu, Jun 28, 2018 at 1:28 PM Al Viro wrote: > > > > > > Sure, but... > > > > static __poll_t binder_poll(struct file *filp, > > struct poll_table_struct *wai

Re: [PATCH 6/6] fs: replace f_ops->get_poll_head with a static ->f_poll_head pointer

On Thu, Jun 28, 2018 at 09:28:37PM +0100, Al Viro wrote: > Sure. Unfortunately, adding yourself to the poll table is not the only > way to block. And a plenty of instances in e.g. drivers/media (where > the bulk of ->poll() instances lives) are very free with grabbing mutexes

Re: [PATCH 6/6] fs: replace f_ops->get_poll_head with a static ->f_poll_head pointer

On Thu, Jun 28, 2018 at 12:31:14PM -0700, Linus Torvalds wrote: > > * a *LOT* of ->poll() instances only block in __pollwait() > > called (indirectly) on the first pass. > > They are *all* supposed to do it. Sure, but... static __poll_t binder_poll(struct file *filp,

Re: [PATCH 6/6] fs: replace f_ops->get_poll_head with a static ->f_poll_head pointer

On Thu, Jun 28, 2018 at 09:40:21AM -0700, Linus Torvalds wrote: > On Thu, Jun 28, 2018 at 7:21 AM Christoph Hellwig wrote: > > > > Note that for this removes the possibility of actually returning an > > error before waiting in poll. We could still do this with an ERR_PTR > > in f_poll_head with

Re: [Patch net] socket: close race condition between sock_close() and sockfs_setattr()

On Thu, Jun 07, 2018 at 03:15:15PM -0700, Cong Wang wrote: > > You do realize that the same ->setattr() can be called by way of > > chown() on /proc/self/fd/, right? What would you do there - > > bump refcount on that struct file when traversing that symlink and > > hold it past the end of

Re: [Patch net] socket: close race condition between sock_close() and sockfs_setattr()

On Thu, Jun 07, 2018 at 02:45:58PM -0700, Cong Wang wrote: > On Thu, Jun 7, 2018 at 2:26 PM, Al Viro wrote: > > On Thu, Jun 07, 2018 at 01:39:49PM -0700, Cong Wang wrote: > >> fchownat() doesn't even hold refcnt of fd until it figures out > >> fd is really n

Re: [Patch net] socket: close race condition between sock_close() and sockfs_setattr()

On Thu, Jun 07, 2018 at 01:39:49PM -0700, Cong Wang wrote: > fchownat() doesn't even hold refcnt of fd until it figures out > fd is really needed (otherwise is ignored) and releases it after > it resolves the path. This means sock_close() could race with > sockfs_setattr(), which leads to a NULL

Re: aio poll and a new in-kernel poll API V13

OK, it's in -next now; there are several cleanups I'd put into vfs.git#work.aio: aio: all callers of aio_{read,write,fsync,poll} treat 0 and -EIOCBQUEUED the same way aio_read_events_ring(): make a bit more readable aio: shift copyin of iocb into io_submit_one()

Re: aio poll and a new in-kernel poll API V13

On Sat, May 26, 2018 at 01:11:11AM +0100, Al Viro wrote: > On Wed, May 23, 2018 at 09:19:49PM +0200, Christoph Hellwig wrote: > > Hi all, > > > > this series adds support for the IOCB_CMD_POLL operation to poll for the > > readyness of file descriptors using the aio s

Re: aio poll and a new in-kernel poll API V13

On Wed, May 23, 2018 at 09:19:49PM +0200, Christoph Hellwig wrote: > Hi all, > > this series adds support for the IOCB_CMD_POLL operation to poll for the > readyness of file descriptors using the aio subsystem. The API is based > on patches that existed in RHAS2.1 and RHEL3, which means it

YAaioRace (was Re: [PATCH 08/31] aio: implement IOCB_CMD_POLL)

On Wed, May 23, 2018 at 01:49:04AM +0100, Al Viro wrote: > > Looks like we want to call ->ki_cancel() *BEFORE* removing from the list, > > as well as doing fput() after aio_complete(). The same ordering, BTW, goes > > for aio_read() et.al. > > > > Look:

Re: [PATCH 08/31] aio: implement IOCB_CMD_POLL

On Wed, May 23, 2018 at 01:45:30AM +0100, Al Viro wrote: > Oh, bugger... > > wakeup > removed from queue > schedule __aio_poll_complete() > > cancel > grab ctx->lock > remove from list > work > aio_complete() >

Re: [PATCH 08/31] aio: implement IOCB_CMD_POLL

On Tue, May 22, 2018 at 11:05:24PM +0100, Al Viro wrote: > > +{ > > + struct aio_kiocb *iocb = container_of(req, struct aio_kiocb, poll); > > + > > + fput(req->file); > > + aio_complete(iocb, mangle_poll(mask), 0); > > +} > > Careful. > &g

Re: aio poll and a new in-kernel poll API V12

On Tue, May 22, 2018 at 01:30:37PM +0200, Christoph Hellwig wrote: > Hi all, > > this series adds support for the IOCB_CMD_POLL operation to poll for the > readyness of file descriptors using the aio subsystem. The API is based > on patches that existed in RHAS2.1 and RHEL3, which means it

Re: [PATCH 08/31] aio: implement IOCB_CMD_POLL

On Tue, May 22, 2018 at 01:30:45PM +0200, Christoph Hellwig wrote: > +static inline void __aio_poll_complete(struct poll_iocb *req, __poll_t mask) > +{ > + struct aio_kiocb *iocb = container_of(req, struct aio_kiocb, poll); > + > + fput(req->file); > + aio_complete(iocb,

Re: [PATCH 10/32] aio: implement IOCB_CMD_POLL

On Sun, May 20, 2018 at 06:32:25AM +0100, Al Viro wrote: > > + spin_lock_irqsave(>ctx_lock, flags); > > + list_add_tail(>ki_list, >delayed_cancel_reqs); > > + spin_unlock(>ctx_lock); > > ... and io_cancel(2) comes, finds it and inhume^Wcompletes it, leav

Re: [PATCH 10/32] aio: implement IOCB_CMD_POLL

On Tue, May 15, 2018 at 09:48:11PM +0200, Christoph Hellwig wrote: > +static ssize_t aio_poll(struct aio_kiocb *aiocb, struct iocb *iocb) > +{ > + struct kioctx *ctx = aiocb->ki_ctx; > + struct poll_iocb *req = >poll; > + unsigned long flags; > + __poll_t mask; > + > + /*

Re: [PATCH 08/32] aio: replace kiocb_set_cancel_fn with a cancel_kiocb file operation

On Tue, May 15, 2018 at 09:48:09PM +0200, Christoph Hellwig wrote: > case -EIOCBQUEUED: > + if (req->ki_filp->f_op->cancel_kiocb) { > + struct aio_kiocb *iocb = > + container_of(req, struct aio_kiocb, rw); > +

Re: simplify procfs code for seq_file instances V3

On Wed, May 16, 2018 at 11:43:04AM +0200, Christoph Hellwig wrote: > We currently have hundreds of proc files that implement plain, read-only > seq_file based interfaces. This series consolidates them using new > procfs helpers that take the seq_operations or simple show callback > directly. > >

Re: [PATCH] coredump: rename umh_pipe_setup() to coredump_pipe_setup()

On Thu, May 10, 2018 at 11:32:47PM +, Luis R. Rodriguez wrote: > I think net-next makes sense if Al Viro is OK with that. This way it could go > in regardless of the state of your series, but it also lines up with your > work. Fine by me...

Re: simplify procfs code for seq_file instances V2

On Sun, May 06, 2018 at 08:19:49PM +0300, Alexey Dobriyan wrote: > +++ b/fs/proc/internal.h > @@ -48,8 +48,8 @@ struct proc_dir_entry { > const struct seq_operations *seq_ops; > int (*single_show)(struct seq_file *, void *); > }; > - unsigned int state_size; >

Re: [bisected] Stack overflow after fs: "switch the IO-triggering parts of umount to fs_pin" (was net namespaces kernel stack overflow)

On Thu, Apr 19, 2018 at 04:34:48PM +0100, Al Viro wrote: > IOW, we only get there if our vfsmount was an MNT_INTERNAL one. > So we have mnt->mnt_umount of some MNT_INTERNAL mount found in > ->mnt_pins of some other mount. Which, AFAICS, means that > it used to be mounted o

Re: [bisected] Stack overflow after fs: "switch the IO-triggering parts of umount to fs_pin" (was net namespaces kernel stack overflow)

On Thu, Apr 19, 2018 at 03:50:25PM +0300, Kirill Tkhai wrote: > Hi, Al, > > commit 87b95ce0964c016ede92763be9c164e49f1019e9 is the first after which the > below test crashes the kernel: > > Author: Al Viro <v...@zeniv.linux.org.uk> > Date: Sat

Re: [PATCH 05/32] fs: introduce new ->get_poll_head and ->poll_mask methods

On Fri, Mar 30, 2018 at 05:07:42PM +0200, Christoph Hellwig wrote: > + get_poll_head: Returns the struct wait_queue_head that poll, select, > + epoll or aio poll should wait on in case this instance only has single > + waitqueue. Can return NULL to indicate polling is not supported, > + or a

Re: [PATCH 08/32] aio: replace kiocb_set_cancel_fn with a cancel_kiocb file operation

On Fri, Mar 30, 2018 at 05:07:45PM +0200, Christoph Hellwig wrote: > The current kiocb_set_cancel_fn implementation assumes the kiocb is > embedded into an aio_kiocb, which is fundamentally unsafe as it might > have been submitted by non-aio callers. Instead add a cancel_kiocb > file operation

Re: [PATCH net 1/2] net: bcmgenet: Fix sparse warnings in bcmgenet_put_tx_csum()

On Tue, Apr 03, 2018 at 12:33:05PM -0400, David Miller wrote: > Yes Al, however the pattern choosen here is probably cheaper on little endian: > > __beXX val = x; > switch (val) { > case htons(ETH_P_FOO): >... > } > > This way only the compiler byte swaps the

Re: [PATCH net 1/2] net: bcmgenet: Fix sparse warnings in bcmgenet_put_tx_csum()

On Mon, Apr 02, 2018 at 03:58:55PM -0700, Florian Fainelli wrote: > skb->protocol is a __be16 which we would be calling htons() against, > while this is not wrong per-se as it correctly results in swapping the > value on LE hosts, this still upsets sparse. Adopt a similar pattern to > what other

Re: [PATCH net 2/2] net: systemport: Fix sparse warnings in bcm_sysport_insert_tsb()

On Mon, Apr 02, 2018 at 03:58:56PM -0700, Florian Fainelli wrote: > skb->protocol is a __be16 which we would be calling htons() against, > while this is not wrong per-se as it correctly results in swapping the > value on LE hosts, this still upsets sparse. Adopt a similar pattern to > what other

Re: WARNING: refcount bug in should_fail

On Mon, Apr 02, 2018 at 10:59:34PM +0100, Al Viro wrote: > FWIW, I'm going through the ->kill_sb() instances, fixing that sort > of bugs (most of them preexisting, but I should've checked instead > of assuming that everything's fine). Will push out later tonight. OK, see vfs.g

Re: WARNING: refcount bug in should_fail

On Mon, Apr 02, 2018 at 10:52:12PM +0100, Al Viro wrote: > On Mon, Apr 02, 2018 at 03:30:56PM -0500, Eric W. Biederman wrote: > > Tetsuo Handa <penguin-ker...@i-love.sakura.ne.jp> writes: > > > > I don't think this is a dup of existing bug. > > > We

Re: WARNING: refcount bug in should_fail

On Mon, Apr 02, 2018 at 03:30:56PM -0500, Eric W. Biederman wrote: > Tetsuo Handa writes: > > I don't think this is a dup of existing bug. > > We need to fix either 9ee332d99e4d5a97 or d91ee87d8d85a080. > > Even if expanding mount_ns to more filesystems was

Re: [PATCH 07/30] aio: add delayed cancel support

On Thu, Mar 29, 2018 at 10:33:05PM +0200, Christoph Hellwig wrote: > The upcoming aio poll support would like to be able to complete the > iocb inline from the cancellation context, but that would cause a > double lock of ctx_lock with the current locking scheme. Move the > cancelation outside

Re: [PATCH 07/30] aio: add delayed cancel support

On Thu, Mar 29, 2018 at 10:53:05AM +0200, Christoph Hellwig wrote: > On Wed, Mar 28, 2018 at 05:35:26PM +0100, Al Viro wrote: > > > ret = vfs_fsync(req->file, req->datasync); > > > - fput(req->file); > > > - aio_complete(container_o

Re: [PATCH 07/30] aio: add delayed cancel support

On Wed, Mar 28, 2018 at 05:35:26PM +0100, Al Viro wrote: > On Wed, Mar 28, 2018 at 09:29:03AM +0200, Christoph Hellwig wrote: > > static void aio_fsync_work(struct work_struct *work) > > { > > struct fsync_iocb *req = container_of(work, struct fsync_iocb, work); >

Re: [PATCH 07/30] aio: add delayed cancel support

On Wed, Mar 28, 2018 at 09:29:03AM +0200, Christoph Hellwig wrote: > static void aio_fsync_work(struct work_struct *work) > { > struct fsync_iocb *req = container_of(work, struct fsync_iocb, work); > + struct aio_kiocb *iocb = container_of(req, struct aio_kiocb, fsync); > + struct

Re: [PATCH 06/28] aio: implement IOCB_CMD_POLL

On Thu, Mar 22, 2018 at 06:24:10PM +0100, Christoph Hellwig wrote: > -static void aio_complete(struct aio_kiocb *iocb, long res, long res2) > +static bool aio_complete(struct aio_kiocb *iocb, long res, long res2, > + unsigned complete_flags) Looks like all callers are following that

Re: [PATCH 06/28] aio: implement IOCB_CMD_POLL

On Wed, Mar 21, 2018 at 08:40:10AM +0100, Christoph Hellwig wrote: > Simple one-shot poll through the io_submit() interface. To poll for > a file descriptor the application should submit an iocb of type > IOCB_CMD_POLL. It will poll the fd for the events specified in the > the first 32 bits of

Re: [PATCH v5 0/2] Remove false-positive VLAs when using max()

On Tue, Mar 20, 2018 at 04:26:52PM -0700, Linus Torvalds wrote: > On Tue, Mar 20, 2018 at 4:23 PM, Linus Torvalds > wrote: > > > > Hmm. So thanks to the diseased mind of Martin Uecker, there's a better > > test for "__is_constant()": > > > > /* Glory to Martin

Re: sctp: use proc_remove_subtree()

On Fri, Mar 16, 2018 at 12:44:15PM -0400, David Miller wrote: > From: Al Viro <v...@zeniv.linux.org.uk> > Date: Wed, 14 Mar 2018 20:51:19 + > > > On Wed, Mar 14, 2018 at 08:46:21PM +, Al Viro wrote: > >> use proc_remove_subtree() for subtree removal, both

[PATCH v2] sctp: use proc_remove_subtree()

use proc_remove_subtree() for subtree removal, both on setup failure halfway through and on teardown. No need to make simple things complex... Signed-off-by: Al Viro <v...@zeniv.linux.org.uk> --- include/net/sctp/sctp.h | 11 +-- net/sctp/objcnt.c | 8 net/sctp/

Re: [PATCH v5 0/2] Remove false-positive VLAs when using max()

On Fri, Mar 16, 2018 at 01:15:27PM -0700, Linus Torvalds wrote: > On Fri, Mar 16, 2018 at 1:12 PM, Al Viro <v...@zeniv.linux.org.uk> wrote: > > > > That's C99, straight from N1256.pdf (C99-TC3)... > > I checked C90, since the error is > >ISO C90 forbids

Re: [PATCH v5 0/2] Remove false-positive VLAs when using max()

On Fri, Mar 16, 2018 at 12:27:23PM -0700, Linus Torvalds wrote: > But it sure isn't "variable" either as far as the standard is > concerned, because the standard doesn't even have that concept (it > uses "variable" for argument numbers and for variables). Huh? 6.7.5.2p4: If the size is not

Re: [PATCH v5 0/2] Remove false-positive VLAs when using max()

On Fri, Mar 16, 2018 at 05:55:02PM +, Al Viro wrote: > On Fri, Mar 16, 2018 at 10:29:16AM -0700, Linus Torvalds wrote: > >t.c: In function ‘test’: > >t.c:6:6: error: argument to variable-length array is too large > > [-Werror=vla-larger-than=] > > int a

Re: [PATCH v5 0/2] Remove false-positive VLAs when using max()

On Fri, Mar 16, 2018 at 10:29:16AM -0700, Linus Torvalds wrote: >t.c: In function ‘test’: >t.c:6:6: error: argument to variable-length array is too large > [-Werror=vla-larger-than=] > int array[(1,100)]; > > Gcc people are crazy. That's not them, that's C standard regarding ICE.

Re: sctp: use proc_remove_subtree()

On Wed, Mar 14, 2018 at 08:46:21PM +, Al Viro wrote: > use proc_remove_subtree() for subtree removal, both on setup failure > halfway through and on teardown. No need to make simple things > complex... ... and sctp_dbg_objcnt_exit() can be removed as well - remove_proc_subtree()

sctp: use proc_remove_subtree()

use proc_remove_subtree() for subtree removal, both on setup failure halfway through and on teardown. No need to make simple things complex... Signed-off-by: Al Viro <v...@zeniv.linux.org.uk> --- include/net/sctp/sctp.h | 9 + net/sctp/proc.c

  1   2   3   4   >