lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 6 Oct 2017 05:09:13 +0000
From:   "Kalderon, Michal" <Michal.Kalderon@...ium.com>
To:     David Miller <davem@...emloft.net>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
        "dledford@...hat.com" <dledford@...hat.com>,
        "Elior, Ariel" <Ariel.Elior@...ium.com>
Subject: Re: [PATCH v2 net-next 06/12] qed: Add LL2 slowpath handling

From: linux-rdma-owner@...r.kernel.org <linux-rdma-owner@...r.kernel.org> on behalf of David Miller <davem@...emloft.net>

>From: "Kalderon, Michal" <Michal.Kalderon@...ium.com>
>Date: Thu, 5 Oct 2017 20:27:22 +0000
>
>> The spinlock is required for the case that rx buffers are posted
>> from a different thread, where it could be run simultaneously to the
>> rxq_completion.
>
>This only brings us back to my original argument, if the lock is
>necessary in order to synchronize with those paths, how can you
>possible drop the lock safely here?
>
>Is it because you re-read the head and tail pointers of the queue each
>time around the loop?

It's safe to drop the lock here because the implementation of the queue (qed_chain)
maintains both a consumer pointer (tail) indices and producer pointer(head).
The main loop reads  the FWs value and stores it as the "new cons"
and traverses the queue only until it reaches the FWs consumer value. 
The post function adds more buffers to the end of the queue and updates the 
producer. They will not affect the consumer pointers. So posting of buffers
doesn't affect the main loop.

The resources that are protected by the lock and accessed inside the loop
and from post-buffers are three linked-lists, free-descq, posting_descq and
active_descq, their head and tail are  read on every access
(elements are removed and moved between the lists).

Following this discussion, it looks like there was no need to take the lock in the
outer function, but only around the places that access these lists, this is a delicate
change which affects the ll2 clients (iscsi,fcoe,roce,iwarp). As this series is rather
large as it is and is intended for iWARP, please consider taking this one as it is. 
Since it doesn't change existing functionality and doesn't introduce risk to other
components. 

thanks,
Michal


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists