RE: [char-misc-next V3] mei: add reference counting for me clients
> -Original Message- > From: Greg KH [mailto:gre...@linuxfoundation.org] > Sent: Thursday, November 06, 2014 17:58 > To: Winkler, Tomas > Cc: a...@arndb.de; linux-kernel@vger.kernel.org > Subject: Re: [char-misc-next V3] mei: add reference counting for me clients > > On Thu, Nov 06, 2014 at 08:40:21AM +, Winkler, Tomas wrote: > > > > > > On Tue, Nov 04, 2014 at 05:22:51AM +, Winkler, Tomas wrote: > > > > > > > > > > On Mon, Nov 03, 2014 at 10:42:05AM +0200, Tomas Winkler wrote: > > > > > > To support dynamic addition/remove we add reference > > > > > > counter. > > > > > > > > > > What is keeping two different threads / cpus from grabbing a reference > > > > > at the same time the other one is dropping it? > > > > > > > > > > You have a list here, with no locking, right? You also don't have any > > > > > locking for your kref, which isn't good at all... > > > > > > > > > > Please audit and fix up. > > > > > > > > Of course there is a lock, it wouldn't work at all. It is not explicit > > > > but we run under device_lock mutex. > > > > > > Please make it explicit :) > > > > Not sure what you mean by that? There is a lot of options in this laconic > sentence. > > In looking at that patch, it was not obvious to me that any locking was > happening at all, so that needs to be addressed somehow before I can > accept the change. I understand the concern though we are using a BIG driver mutex so no additional locking is required, we are already locked. Currently we didn't hit any performance issue that required more fine grained locking but let say this is something we aware and that might happen in the future. Thanks Tomas -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [char-misc-next V3] mei: add reference counting for me clients
On Thu, Nov 06, 2014 at 08:40:21AM +, Winkler, Tomas wrote: > > > > On Tue, Nov 04, 2014 at 05:22:51AM +, Winkler, Tomas wrote: > > > > > > > > On Mon, Nov 03, 2014 at 10:42:05AM +0200, Tomas Winkler wrote: > > > > > To support dynamic addition/remove we add reference > > > > > counter. > > > > > > > > What is keeping two different threads / cpus from grabbing a reference > > > > at the same time the other one is dropping it? > > > > > > > > You have a list here, with no locking, right? You also don't have any > > > > locking for your kref, which isn't good at all... > > > > > > > > Please audit and fix up. > > > > > > Of course there is a lock, it wouldn't work at all. It is not explicit > > > but we run under device_lock mutex. > > > > Please make it explicit :) > > Not sure what you mean by that? There is a lot of options in this laconic > sentence. In looking at that patch, it was not obvious to me that any locking was happening at all, so that needs to be addressed somehow before I can accept the change. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [char-misc-next V3] mei: add reference counting for me clients
> > On Tue, Nov 04, 2014 at 05:22:51AM +, Winkler, Tomas wrote: > > > > > > On Mon, Nov 03, 2014 at 10:42:05AM +0200, Tomas Winkler wrote: > > > > To support dynamic addition/remove we add reference > > > > counter. > > > > > > What is keeping two different threads / cpus from grabbing a reference > > > at the same time the other one is dropping it? > > > > > > You have a list here, with no locking, right? You also don't have any > > > locking for your kref, which isn't good at all... > > > > > > Please audit and fix up. > > > > Of course there is a lock, it wouldn't work at all. It is not explicit > > but we run under device_lock mutex. > > Please make it explicit :) Not sure what you mean by that? There is a lot of options in this laconic sentence. Thanks Tomas -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [char-misc-next V3] mei: add reference counting for me clients
On Thu, Nov 06, 2014 at 08:40:21AM +, Winkler, Tomas wrote: On Tue, Nov 04, 2014 at 05:22:51AM +, Winkler, Tomas wrote: On Mon, Nov 03, 2014 at 10:42:05AM +0200, Tomas Winkler wrote: To support dynamic addition/remove we add reference counter. What is keeping two different threads / cpus from grabbing a reference at the same time the other one is dropping it? You have a list here, with no locking, right? You also don't have any locking for your kref, which isn't good at all... Please audit and fix up. Of course there is a lock, it wouldn't work at all. It is not explicit but we run under device_lock mutex. Please make it explicit :) Not sure what you mean by that? There is a lot of options in this laconic sentence. In looking at that patch, it was not obvious to me that any locking was happening at all, so that needs to be addressed somehow before I can accept the change. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [char-misc-next V3] mei: add reference counting for me clients
-Original Message- From: Greg KH [mailto:gre...@linuxfoundation.org] Sent: Thursday, November 06, 2014 17:58 To: Winkler, Tomas Cc: a...@arndb.de; linux-kernel@vger.kernel.org Subject: Re: [char-misc-next V3] mei: add reference counting for me clients On Thu, Nov 06, 2014 at 08:40:21AM +, Winkler, Tomas wrote: On Tue, Nov 04, 2014 at 05:22:51AM +, Winkler, Tomas wrote: On Mon, Nov 03, 2014 at 10:42:05AM +0200, Tomas Winkler wrote: To support dynamic addition/remove we add reference counter. What is keeping two different threads / cpus from grabbing a reference at the same time the other one is dropping it? You have a list here, with no locking, right? You also don't have any locking for your kref, which isn't good at all... Please audit and fix up. Of course there is a lock, it wouldn't work at all. It is not explicit but we run under device_lock mutex. Please make it explicit :) Not sure what you mean by that? There is a lot of options in this laconic sentence. In looking at that patch, it was not obvious to me that any locking was happening at all, so that needs to be addressed somehow before I can accept the change. I understand the concern though we are using a BIG driver mutex so no additional locking is required, we are already locked. Currently we didn't hit any performance issue that required more fine grained locking but let say this is something we aware and that might happen in the future. Thanks Tomas -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [char-misc-next V3] mei: add reference counting for me clients
On Tue, Nov 04, 2014 at 05:22:51AM +, Winkler, Tomas wrote: On Mon, Nov 03, 2014 at 10:42:05AM +0200, Tomas Winkler wrote: To support dynamic addition/remove we add reference counter. What is keeping two different threads / cpus from grabbing a reference at the same time the other one is dropping it? You have a list here, with no locking, right? You also don't have any locking for your kref, which isn't good at all... Please audit and fix up. Of course there is a lock, it wouldn't work at all. It is not explicit but we run under device_lock mutex. Please make it explicit :) Not sure what you mean by that? There is a lot of options in this laconic sentence. Thanks Tomas -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [char-misc-next V3] mei: add reference counting for me clients
On Tue, Nov 04, 2014 at 05:22:51AM +, Winkler, Tomas wrote: > > > > On Mon, Nov 03, 2014 at 10:42:05AM +0200, Tomas Winkler wrote: > > > To support dynamic addition/remove we add reference > > > counter. > > > > What is keeping two different threads / cpus from grabbing a reference > > at the same time the other one is dropping it? > > > > You have a list here, with no locking, right? You also don't have any > > locking for your kref, which isn't good at all... > > > > Please audit and fix up. > > Of course there is a lock, it wouldn't work at all. It is not explicit > but we run under device_lock mutex. Please make it explicit :) thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [char-misc-next V3] mei: add reference counting for me clients
On Tue, Nov 04, 2014 at 05:22:51AM +, Winkler, Tomas wrote: On Mon, Nov 03, 2014 at 10:42:05AM +0200, Tomas Winkler wrote: To support dynamic addition/remove we add reference counter. What is keeping two different threads / cpus from grabbing a reference at the same time the other one is dropping it? You have a list here, with no locking, right? You also don't have any locking for your kref, which isn't good at all... Please audit and fix up. Of course there is a lock, it wouldn't work at all. It is not explicit but we run under device_lock mutex. Please make it explicit :) thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [char-misc-next V3] mei: add reference counting for me clients
> > On Mon, Nov 03, 2014 at 10:42:05AM +0200, Tomas Winkler wrote: > > To support dynamic addition/remove we add reference > > counter. > > What is keeping two different threads / cpus from grabbing a reference > at the same time the other one is dropping it? > > You have a list here, with no locking, right? You also don't have any > locking for your kref, which isn't good at all... > > Please audit and fix up. Of course there is a lock, it wouldn't work at all. It is not explicit but we run under device_lock mutex. Tomas -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [char-misc-next V3] mei: add reference counting for me clients
On Mon, Nov 03, 2014 at 10:42:05AM +0200, Tomas Winkler wrote: > To support dynamic addition/remove we add reference > counter. What is keeping two different threads / cpus from grabbing a reference at the same time the other one is dropping it? You have a list here, with no locking, right? You also don't have any locking for your kref, which isn't good at all... Please audit and fix up. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [char-misc-next V3] mei: add reference counting for me clients
On Mon, Nov 03, 2014 at 10:42:05AM +0200, Tomas Winkler wrote: To support dynamic addition/remove we add reference counter. What is keeping two different threads / cpus from grabbing a reference at the same time the other one is dropping it? You have a list here, with no locking, right? You also don't have any locking for your kref, which isn't good at all... Please audit and fix up. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
RE: [char-misc-next V3] mei: add reference counting for me clients
On Mon, Nov 03, 2014 at 10:42:05AM +0200, Tomas Winkler wrote: To support dynamic addition/remove we add reference counter. What is keeping two different threads / cpus from grabbing a reference at the same time the other one is dropping it? You have a list here, with no locking, right? You also don't have any locking for your kref, which isn't good at all... Please audit and fix up. Of course there is a lock, it wouldn't work at all. It is not explicit but we run under device_lock mutex. Tomas -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
