Arnd Bergmann wrote on Mon, Apr 19, 2021 at 02:16:36PM +0200:
> In some cases, you can use the device_link infrastructure to deal
> with dependencies between devices. Not sure if this would help
> in your case, but have a look at device_link_add() etc in drivers/base/core.c
I'll need to actually t
Geert Uytterhoeven wrote on Mon, Apr 19, 2021 at 11:03:24AM +0200:
> > This is going to need quite some more work to be acceptable, in my
> > opinion, but I think it should be possible.
>
> In general, this is very hard to do, IMHO. Some drivers may be used on
> multiple platforms, some of them
Alice Guo (OSS) wrote on Mon, Apr 19, 2021 at 12:27:22PM +0800:
> From: Alice Guo
>
> Update all the code that use soc_device_match
A single patch might be difficult to accept for all components, a each
maintainer will probably want to have a say on their subsystem?
I would suggest to split the
First comment overall for the whole serie:
Since it is the solution I had suggested when I reported the problem[1]
I have no qualm on the approach, comments for individual patches
follow.
[1] http://lore.kernel.org/r/YGGZJjAxA1IO+/v...@atmark-techno.com
Alice Guo (OSS) wrote on Mon, Apr 19, 2021
Jisheng Zhang wrote on Tue, Mar 02, 2021 at 03:39:40PM +0800:
> > Rather than make an exception for 0, how about just removing the if as
> > follow ?
>
> IMHO, we may need to keep the "if" in current logic. When count
> reaches zero, we need to break the "while(iov_iter_count(to))" loop, so
> rem
Jisheng Zhang wrote on Mon, Mar 01, 2021 at 11:01:57AM +0800:
> Per my understanding of iov_iter, we need to call iov_iter_advance()
> even when the read out count is 0. I believe we can see this common style
> in other fs.
I'm not sure where you see this style, but I don't see exceptions for
0-si
Jisheng Zhang wrote on Mon, Mar 01, 2021 at 10:33:36AM +0800:
> I met below warning when cating a small size(about 80bytes) txt file
> on 9pfs(msize=2097152 is passed to 9p mount option), the reason is we
> miss iov_iter_advance() if the read count is 0, so we didn't truncate
> the pipe, then iov_i
YANG LI wrote on Wed, Dec 30, 2020:
> The pointer p is being used but it isn't being initialized,
> need to assign a NULL to it.
My understanding is p has to be initialized: the only way out of the
while(1) loop below sets it.
I don't mind fixing false positive warnings as it makes maintenance
e
the future)
Dan Carpenter (2):
9p: Uninitialized variable in v9fs_writeback_fid()
9p: Remove unnecessary IS_ERR() check
Dominique Martinet (2):
9p: apply review requests for fid refcounting
9p: Fix writeback fid incorrectly being a
Andrew Lunn wrote on Sun, Nov 01, 2020:
> > > Signed-off-by: Andrew Lunn
Acked-by: Dominique Martinet
> >
> > Thanks, LGTM I'll take this for next cycle unless someone is grabbing
> > these
>
> I hope to turn on W=1 by default soon in most of /net. Th
Andrew Lunn wrote on Sat, Oct 31, 2020:
> net/9p/client.c:420: warning: Function parameter or member 'c' not described
> in 'p9_client_cb'
> net/9p/client.c:420: warning: Function parameter or member 'req' not
> described in 'p9_client_cb'
> net/9p/client.c:420: warning: Function parameter or mem
Hi Linus,
another harmless cycle.
(sorry latest commit's message isn't great, I was half expecting a v2
but it didn't come and I remembered too late/didn't want to reword it
myself; and it's still worth taking as is)
Thanks,
The following changes since commit 549738f15da0e5a00275977623be199fb
Anant Thazhemadam wrote on Mon, Oct 12, 2020:
> You mentioned how a fully zeroed address isn't exactly faulty. By extension,
> wouldn't that
> mean that an address that simply begins with a 0 isn't faulty as well?
That is correct.
If you have a look at the unix(7) man page that describes AF_UNIX,
Anant Thazhemadam wrote on Mon, Oct 12, 2020:
> In p9_fd_create_unix, checking is performed to see if the addr (passed
> as an argument) is NULL or not.
> However, no check is performed to see if addr is a valid address, i.e.,
> it doesn't entirely consist of only 0's.
> The initialization of sun_s
Mark Brown wrote on Wed, Aug 26, 2020:
> On Wed, Aug 26, 2020 at 03:38:15AM -0700, syzbot wrote:
>
> > console output: https://syzkaller.appspot.com/x/log.txt?x=10615b3690
> > kernel config: https://syzkaller.appspot.com/x/.config?x=a61d44f28687f508
> > dashboard link: https://syzkaller.appsp
Hi Linus,
the usual small stuff.
Thanks!
The following changes since commit 74d6a5d5662975aed7f25952f62efbb6f6dadd29:
9p/trans_fd: Fix concurrency del of req_list in p9_fd_cancelled/p9_read_work
(2020-07-19 14:58:47 +0200)
are available in the Git repository at:
https://github.com/mar
Alexey Kardashevskiy wrote on Thu, Aug 06, 2020:
> I am seeing another bug in 9p under syzkaller, the reprocase is:
>
> r0 = open$dir(&(0x7f40)='./file0\x00', 0x88142, 0x182)
>
> r1 = openat$null(0xff9c, &(0x7f000640)='/dev/null\x00',
> 0x0, 0x0)
> mount$9p_fd(0x0, &(0x7f0
Sorry for the late request, I took some time to fix my test setup and to
be convinced these two patches are worth sending now and not wait until
the next merge window.
(the "weird" -2 at the end of the tag is because I had already used
9p-for-5.8 for the original -rc1 pull)
The following changes
Greg Kurz wrote on Tue, Jul 28, 2020:
> > The "fd" transport layer uses 2 file descriptors passed externally
> > and calls kernel_write()/kernel_read() on these. If files were opened
> > without FMODE_WRITE/FMODE_READ, WARN_ON_ONCE() will fire.
There already is a fix in linux-next as a39c46067c84
David Miller wrote on Wed, Jul 15, 2020:
> From: Dominique Martinet
> Date: Wed, 15 Jul 2020 15:47:56 +0200
> > It's honestly just a warn on something that would fail anyway so I'd
> > rather let it live in -next first, I don't get why syzbot is so verbose
> &
Christoph Hellwig wrote on Wed, Jul 15, 2020:
> FYI, this is now generating daily syzbot reports, so I'd love to see
> the fix going into Linus' tree ASAP..
Yes, I'm getting some syzbot warnings as well now.
I had however only planned to get this in linux-next, since that is what
the syzbot mails
Andrew Lunn wrote on Mon, Jul 13, 2020:
> Simple fixes which require no deep knowledge of the code.
>
> Cc: Eric Van Hensbergen
> Cc: Latchesar Ionkov
> Cc: Dominique Martinet
Ack.
> Signed-off-by: Andrew Lunn
Can't hurt & looks correct; thanks.
David, assuming y
Doug Nazar wrote on Fri, Jul 10, 2020:
> On 2020-07-10 04:57, Christoph Hellwig wrote:
>
> >diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c
> >index 13cd683a658ab6..1cd8ea0e493617 100644
> >--- a/net/9p/trans_fd.c
> >+++ b/net/9p/trans_fd.c
> >@@ -803,20 +803,28 @@ static int p9_fd_open(struct
Christoph Hellwig wrote on Fri, Jul 10, 2020:
> p9_fd_open just fgets file descriptors passed in from userspace, but
> doesn't verify that they are valid for read or writing. This gets
> cought down in the VFS when actually attemping a read or write, but a
> new warning added in linux-next upsets
Alexander Kapshuk wrote on Sun, Jun 21, 2020:
> Fix rcu not being dereferenced cleanly by using the task
> helpers (un)lock_task_sighand instead of spin_lock_irqsave and
> spin_unlock_irqrestore to ensure current->sighand is a valid pointer as
> suggested in the email referenced below.
Ack.
I'll t
Alexander Kapshuk wrote on Sun, Jun 21, 2020:
> Thanks for your feedback.
> Shall I simply resend the v2 patch with the commit message reworded as
> you suggested, or should I make it a v3 patch instead?
Yes please resend the same commit reworded. v2 or v3 doesn't matter
much, the [PATCH whatever]
Alexander Kapshuk wrote on Sat, Jun 20, 2020:
> Use (un)lock_task_sighand instead of spin_lock_irqsave and
> spin_unlock_irqrestore to ensure current->sighand is a valid pointer as
> suggested in the email referenced below.
Thanks for v2! Patch itself looks good to me.
I always add another `Link:
Alexander Kapshuk wrote on Thu, Jun 18, 2020:
> Address sparse endian warning:
> net/9p/trans_fd.c:932:28: warning: incorrect type in assignment (different
> base types)
> net/9p/trans_fd.c:932:28:expected restricted __be32 [addressable]
> [assigned] [usertype] s_addr
> net/9p/trans_fd.c:932:
Alexander Kapshuk wrote on Thu, Jun 18, 2020:
> Address sparse nonderef rcu warnings:
> net/9p/client.c:790:17: warning: incorrect type in argument 1 (different
> address spaces)
> net/9p/client.c:790:17:expected struct spinlock [usertype] *lock
> net/9p/client.c:790:17:got struct spinlock
Wang Hai wrote on Fri, Jun 12, 2020:
> p9_read_work and p9_fd_cancelled may be called concurrently.
> In some cases, req->req_list may be deleted by both p9_read_work
> and p9_fd_cancelled.
>
> We can fix it by ignoring replies associated with a cancelled
> request and ignoring cancelled request i
wanghai (M) wrote on Fri, Jun 12, 2020:
> You are right, I got a syzkaller bug.
>
> "p9_read_work+0x7c3/0xd90" points to list_del(&m->rreq->req_list);
>
> [ 62.733598] kasan: CONFIG_KASAN_INLINE enabled
> [ 62.734484] kasan: GPF could be caused by NULL-ptr deref or user memory
> access
> [
Wang Hai wrote on Thu, Jun 11, 2020:
> p9_read_work and p9_fd_cancelled may be called concurrently.
Good catch. I'm sure this fixes some of the old syzbot bugs...
I'll check other transports handle this properly as well.
> Before list_del(&m->rreq->req_list) in p9_read_work is called,
> the req->
YueHaibing wrote on Tue, Apr 30, 2019:
> If xenbus_register_frontend() fails in p9_trans_xen_init,
> we should call v9fs_unregister_trans() to do cleanup.
>
> Fixes: 868eb122739a ("xen/9pfs: introduce Xen 9pfs transport driver")
> Signed-off-by: YueHaibing
Thanks, queued both in my repo - sorry
zhengbin wrote on Wed, Mar 13, 2019:
> If msize is less than 4096, we should close and put trans, destroy
> tagpool, not just free client. This patch fixes that.
>
> Fixes: 574d356b7a02 ("9p/net: put a lower bound on msize")
> Reported-by: Hulk Robot
> Signed-off-by: zhengbin
Ack, definitely. T
Tom Herbert wrote on Fri, Feb 22, 2019:
> > > So basically it sounds like you're interested in supporting TCP
> > > connections that are half closed. I believe that the error in half
> > > closed is EPIPE, so if the TCP socket returns that it can be ignored
> > > and the socket can continue being a
Tom Herbert wrote on Wed, Feb 20, 2019:
> > When the client closes the socket, some messages are obviously still "in
> > flight", and the server will recv a POLLERR notification on the csock at
> > some point with many messages left.
> > The documentation says to unattach the csock when you get POL
Dominique Martinet wrote on Fri, Feb 15, 2019:
> With all that said I guess my patch should work correctly then, I'll try
> to find some time to check the error does come back up the tcp socket in
> my reproducer but I have no reason to believe it doesn't.
Ok, so I can conf
Tom Herbert wrote on Thu, Feb 14, 2019:
> On Thu, Feb 14, 2019 at 7:31 PM Dominique Martinet
> wrote:
> > Yes, the parser fails with -ENOMEM ; that is not handled gracefully at
> > all: from a user point of view, the connection just hangs (recvmsg never
> > returns), with
Tom Herbert wrote on Thu, Feb 14, 2019:
> > This second patch[2] (the current thread) now does an extra clone if
> > there is an offset, but the problem really isn't in the clone but the
> > pull itself that can fail and return NULL when there is memory pressure.
> > For some reason I hadn't been a
Tom Herbert wrote on Thu, Feb 14, 2019:
> > The best alternative I see is adding a proper helper to get
> > "kcm_rx_msg(skb)->offset" from bpf and document it so users aren't as
> > lost as I have been; I'm not quite sure how/where to add such a helper
> > though as I've barely looked at the bpf co
Dominique Martinet wrote on Wed, Oct 31, 2018:
> Anyway, that probably explains I have no problem with bigger VM
> (uselessly more memory available) or without KASAN (I guess there's
> overhead?), but I'm sending at most 300k of data and the VM has a 1.5GB
> of ram, so if
Gustavo A. R. Silva wrote on Wed, Jan 23, 2019:
> In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> where we are expecting to fall through.
>
> This patch fixes the following warning:
>
> net/9p/trans_xen.c:514:6: warning: this statement may fall through
> [-Wimplicit-fallth
small msize
Adeodato Simó (1):
net/9p: include trans_common.h to fix missing prototype warning.
Dominique Martinet (1):
9p/net: put a lower bound on msize
net/9p/client.c | 21 +
net/9p
Tomas Bortoli wrote on Mon, Dec 17, 2018:
> sorry for the delay, I've been quite busy these days.
No problem.
> The patches looks good to me and should indeed speed up the code a bit.
> I quickly tested them against Syzkaller tuned for the 9p subsystem and
> everything seems fine.
Thanks, can I
From: Dominique Martinet
Add p9_client_async_rpc that will let us send an rpc without waiting
for the reply, as well as an async handler hook.
The work done in the hook could mostly be done in the client callback,
but I prefered not to for a couple of reasons:
- the callback can be called in
From: Dominique Martinet
clunk is defined as making the fid invalid whatever the server returns,
and we should ignore errors so it is a good candidate for async call.
The change should make 9p slightly faster (many vfs systeme calls won't
need to wait for that clunk), but more importantl
From: Dominique Martinet
Make the client flush asynchronous so we don't need to ignore signals in
rpc functions:
- on signal, send a flush request as we previously did but use the new
asynchronous helper and return immediately
- when the reply has been received or connection is destroyed,
Dmitry Vyukov wrote on Thu, Oct 11, 2018:
> > Now we are talking!
> > We generally assume that all modules are simply compiled into kernel.
> > At least that's we have on syzbot. If somebody can't compile them in,
> > we can suggest to add modprobe into init.
> > So this boils down to just writing
Dmitry Vyukov wrote on Thu, Oct 11, 2018:
> But again we don't need to support all of the available hardware.
I agree with that, I just have no idea what the "librxe-rdmav16.so" lib
could be doing and described something I am slightly more familiar with
(e.g. libmlx5)
I talked about a common subse
Dmitry Vyukov wrote on Thu, Oct 11, 2018:
> > That's still the tricky part, I'm afraid... Making a separate server
> > would have been easy because I could have reused some of my junk for the
> > actual connection handling (some rdma helper library I wrote ages
> > ago[1]), but if you're going to j
Dominique Martinet wrote on Wed, Oct 10, 2018:
> It works though, is it just picky because I didn't end it in .git? let's
> try again, sorry for the noise...
>
> #syz test: git://github.com/martinetd/linux.git
> e4ca13f7d075e551dc158df6af18fb412a1dba0a
And I guess the c
Dan Carpenter wrote on Wed, Sep 26, 2018:
> p9_tag_alloc() is supposed to return error pointers, but we accidentally
> return a NULL here. It would cause a NULL dereference in the caller.
>
> Fixes: 996d5b4db4b1 ("9p: Use a slab for allocating requests")
> Signed-off-by: Dan Carpenter
Good catc
bout
this kind of pattern nowadays?
The struct is cleanly zeroed before being read so there is no risk of
double-frees between iterations so zeroing pointers is not strictly
required, but it does make things safer in general.
--
Dominique Martinet
From: Dominique Martinet
If the xen bus exists but does not expose the proper interface, it is
possible to get a non-zero length but still some error, leading to
strcmp failing trying to load invalid memory addresses e.g.
fffe.
There is then no need to check length when there is no
ey'd be ok with that... Ultimately it's probably closer to a design
choice than anything else.
I'll still send a v0 of the patch as is, because I feel it's easier to
understand that we pull because the existing parse_msg functions do not
handle it properly, and will write a note that I intend to move it up a
few lines as a comment.
Thanks,
--
Dominique Martinet
Dominique Martinet wrote on Sun, Aug 05, 2018:
> It's getting late but I'll try adding a pskb_pull in there tomorrow, it
> would be better to make the bpf program start with an offset but I don't
> think that'll be easy to change...
I can confirm the following patch f
Dominique Martinet wrote on Sun, Aug 05, 2018:
> (I'm not sure about offset, since we pass the full skb to parse message,
> wouldn't it look at the start of the buffer everytime? Well, offset
> seems to be 0 everytime the first time that check fails so I can
> probably ignor
Dominique Martinet wrote on Sat, Aug 04, 2018:
> I talked too fast, I can get this to fail on later packets e.g.
> Got 18, expected 31 on 452nd message: 453453453453453453; flags: 80
>
> The content is 453 in a loop so this really is the 453rd packet...
>
> But being slow
Dominique Martinet wrote on Sat, Aug 04, 2018:
> Actually, now I'm looking closer to the timing, it looks specific to the
> connection setup. This send loop works:
> int i = 1;
> while(i <= 1000) {
> int len = (i++
Tom Herbert wrote on Fri, Aug 03, 2018:
> On Fri, Aug 3, 2018 at 4:20 PM, Dominique Martinet
> wrote:
> > Tom Herbert wrote on Fri, Aug 03, 2018:
> >> struct my_proto {
> >>struct _hdr {
> >>uint32_t len;
> >> } hdr;
>
.
(Just to make sure I did fix it to htonl(load_word()) and I can confirm
there is no difference)
Thanks,
--
Dominique Martinet
message.
This happens even if I reduce the VMs CPU to 1, so I was thinking some
irq messes with the sock between skb_peek and the actual copy of the
data (as this deos work if I send slowly!), but even disabling
irq/preempt doesn't seem to help so I'm not sure what to try next.
Any i
outlen);
> }
> -
> +
> /*
>* Take care of in data
>* For example TREAD have 11.
> diff --git a/net/9p/util.c b/net/9p/util.c
> index 59f278e64f58..55ad98277e85 100644
> --- a/net/9p/util.c
> +++ b/net/9p/util.c
> @@ -138,4 +138,3 @@ int p9_idpool_check(int id, struct p9_idpool *p)
> return idr_find(&p->pool, id) != NULL;
> }
> EXPORT_SYMBOL(p9_idpool_check);
> -
--
Dominique Martinet
ks state table could be fixed with introducing a new
> state, but that part is exposed to userspace (ctnetlink) and ugly
> compatibility code would be required for backward compatibility.
I agree a new state is more work than it is worth, I'm happy to leave it
as is.
--
Dominique Martinet | Asmadeus
Dominique Martinet wrote on Wed, Apr 18, 2018:
> Jozsef Kadlecsik wrote on Wed, Apr 18, 2018:
> > Yes, the state transition is wrong for simultaneous open, because the
> > tcp_conntracks table is not (cannot be) smart enough. Could you verify the
> > next untested patch
ke it today
so will report tomorrow)
--
Dominique Martinet | Asmadeus
ale resets.
At least peeling the logs myself helped me follow the process, I'll
sprinkle some carefully crafted logs tomorrow to check if this is true
and will let you figure what is best of trying to preserve scale if it
was set before, setting a default to 14 or something else.
Thanks!
--
Dominique Martinet | Asmadeus
Dominique Martinet wrote on Mon, Apr 16, 2018:
> . . . Oh, there is something interesting there, the connection doesn't
> come up with -G?
Hm, sorry, I take this last part back. I cannot reproduce -G not working
reliably.
I'll dig around the conntrack table a bit more.
--
Do
nntrack-tools): 0 flow entries have been shown.
So something happened that makes it show up in -L (table dump) but not
when querying...?
And only when there is enough traffic: I have previously kept such a
connection without workaround for hours just fine as long as I made sure
not to display more than a screen at a time.
Thanks again,
--
Dominique Martinet | Asmadeus
.9Mbps delivery_rate 431.4Kbps
app_limited busy:36064ms unacked:1 retrans:0/6 rcv_rtt:9112.95
rcv_space:29233 rcv_ssthresh:39184 minrtt:25.566
So I guess lost:33 matching unacked:33 might be another hint?
I'll need a bit more time reading the code to understand what this all
Eric Dumazet wrote on Fri, Apr 13, 2018:
> Ah sorry, your trace was truncated, we need more packets _before_ the excerpt.
Ah, sorry as well. I tried to go far back enough to include the replayed
packets, but I see it didn't include the original ack for that packet.
I'm resending both full traces f
[nop,nop,TS val 1617137954
ecr 1313937714], length 36
16:49:42.165044 IP .13317 > .31872: Flags
[.], seq 32004:33378, ack 4190, win 307, options [nop,nop,TS val 1313953050 ecr
1617129473], length 1374
16:49:42.165073 IP .31872 > .13317: Flags
[.], ack 77346, win 1444, options [nop,nop,TS val 1617144917 ecr
1313937714,nop,nop,sack 1 {32004:33378}], length 0
16:49:43.394773 IP .31872 > .13317: Flags
[P.], seq 4190:4226, ack 77346, win 1444, options [nop,nop,TS val 1617146146
ecr 1313937714], length 36
Once again, thank you both.
--
Dominique Martinet | Asmadeus
Note this is mostly me talking to myself, but in case anyone else hits
the same issues I might as well post my meagre progress.
Dominique Martinet wrote on Fri, Apr 06, 2018:
> (current kernel: vanilla 4.14.29)
reproduced in a fedora VM on that host with a 4.16.1-300.fc28.x86_64
kernel, si
x27;t take the client acks into
account, despite having received that precise 33378 ack earlier and I
believe it should accept a higher ack value anyway ?
The ultimate question being, how can I go about debugging that?
I'm working on getting perf probe/crash to work on the server so I can
look at the kernel side now, I can reproduce this semi-easily so I'm
sure I'll get down to it eventually, but if anyone has an idea I'm all
ears.
Thanks!
--
Dominique Martinet | Asmadeus
Christoph Hellwig wrote on Thu, Mar 03, 2016:
> New version with the nits fixed below. Now that checkpath started
> a stupid warning about not using tabs for indentation which I've
> ignored here and will take up in my usual fights against Joes
> idicotic opinions separately..
Thanks for the nitp
only two I'm aware
of but there might be more (using ganesha myself; happy to help you set
it up in private if you need)
[1] https://github.com/chaos/diod
[2] https://github.com/nfs-ganesha/nfs-ganesha
--
Dominique Martinet
req->rc is pre-allocated early on with p9_tag_alloc and shouldn't be missing
Signed-off-by: Dominique Martinet
---
net/9p/trans_fd.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
Feel free to adapt error code/message if you can think of something better.
diff --g
Eric Van Hensbergen wrote on Sat, Sep 05, 2015:
> On Thu, Sep 3, 2015 at 4:38 AM, Dominique Martinet
> wrote:
> > To be honest, I think it might be better to just bail out if we get in
> > this switch (m->req->rc == NULL after p9_tag_lookup) and not try to
> > al
That code really should never be called (rc is allocated in
tag_alloc), but if it had been it couldn't have worked...
Signed-off-by: Dominique Martinet
---
net/9p/trans_fd.c | 3 +++
1 file changed, 3 insertions(+)
To be honest, I think it might be better to just bail out if we get in
Signed-Off-By: Dominique Martinet
---
net/9p/trans_fd.c | 75 +--
1 file changed, 40 insertions(+), 35 deletions(-)
It ended up alot bigger than I thought it'd be, submitting it anyway
but happy with either version - letting Eric decide w
80 matches
Mail list logo
