Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <9d4c7f38-6a30-3efb-7887-9c6224494ffd@I-love.SAKURA.ne.jp>
Date:   Sun, 4 Sep 2022 22:03:49 +0900
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.1
Subject: Re: [PATCH] net/9p: use a dedicated spinlock for trans_fd
Content-Language: en-US
To:     Dominique Martinet <asmadeus@codewreck.org>,
        v9fs-developer@lists.sourceforge.net
Cc:     syzkaller-bugs@googlegroups.com, linux_oss@crudebyte.com,
        linux-kernel@vger.kernel.org,
        syzbot <syzbot+2f20b523930c32c160cc@syzkaller.appspotmail.com>
References: <2470e028-9b05-2013-7198-1fdad071d999@I-love.SAKURA.ne.jp>
 <20220904112928.1308799-1-asmadeus@codewreck.org>
From:   Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
In-Reply-To: <20220904112928.1308799-1-asmadeus@codewreck.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 2022/09/04 20:29, Dominique Martinet wrote:
> Since the locks actually protect different things in client.c and in
> trans_fd.c, just replace trans_fd.c's lock by a new one specific to the
> transport instead of using spin_lock_irq* variants everywhere
> (client.c's protect the idr for tag allocations, while
> trans_fd.c's protects its own req list and request status field
> that acts as the transport's state machine)

OK, I figured out what this patch changes.

  $ grep -nF -- '->lock' *.[ch]
  client.c:286:   spin_lock_irq(&c->lock);
  client.c:293:   spin_unlock_irq(&c->lock);
  client.c:367:   spin_lock_irqsave(&c->lock, flags);
  client.c:369:   spin_unlock_irqrestore(&c->lock, flags);
  client.c:816:   spin_lock_irq(&clnt->lock);
  client.c:819:   spin_unlock_irq(&clnt->lock);
  client.c:838:   spin_lock_irqsave(&clnt->lock, flags);
  client.c:840:   spin_unlock_irqrestore(&clnt->lock, flags);
  client.c:945:   spin_lock_init(&clnt->lock);
  trans_virtio.c:139:     spin_lock_irqsave(&chan->lock, flags);
  trans_virtio.c:151:     spin_unlock_irqrestore(&chan->lock, flags);
  trans_virtio.c:268:     spin_lock_irqsave(&chan->lock, flags);
  trans_virtio.c:287:                     spin_unlock_irqrestore(&chan->lock, flags);
  trans_virtio.c:296:                     spin_unlock_irqrestore(&chan->lock, flags);
  trans_virtio.c:303:     spin_unlock_irqrestore(&chan->lock, flags);
  trans_virtio.c:474:     spin_lock_irqsave(&chan->lock, flags);
  trans_virtio.c:515:                     spin_unlock_irqrestore(&chan->lock, flags);
  trans_virtio.c:524:                     spin_unlock_irqrestore(&chan->lock, flags);
  trans_virtio.c:532:     spin_unlock_irqrestore(&chan->lock, flags);
  trans_virtio.c:621:     spin_lock_init(&chan->lock);
  trans_xen.c:142:        spin_lock_irqsave(&ring->lock, flags);
  trans_xen.c:149:                spin_unlock_irqrestore(&ring->lock, flags);
  trans_xen.c:164:        spin_unlock_irqrestore(&ring->lock, flags);
  trans_xen.c:314:        spin_lock_init(&ring->lock);

This patch changes "struct p9_client"->lock to be used for only
protecting idr, as explained at

 * @lock: protect @fids and @reqs

line. Makes sense and looks correct.

> Tetsuo Handa-san, thank you very much!
> I'm sorry for not respecting your opinion but it's been a pleasure to
> have submissions from someone on JST :)

Thank you for responding quickly. :-)