From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vladislav Bolkhovitin Subject: Re: [PATCH 8/19]: SCST SYSFS interface implementation Date: Sat, 13 Nov 2010 20:20:18 +0300 Message-ID: <4CDEC8D2.8080101@vlnb.net> References: <20101022175624.GA13640@kroah.com> <4CC1DAA2.7030602@vlnb.net> <20101022185437.GA9103@kroah.com> <4CD8566D.1020202@vlnb.net> <20101109002829.GA22633@kroah.com> <4CD9A9B8.70708@vlnb.net> <4CDA6CD4.3010308@panasas.com> <4CDAFE6E.7050200@vlnb.net> <4CDBBE80.40908@panasas.com> <4CDC56F9.9040601@vlnb.net> <20101112012315.GE17097@core.coreip.homeip.net> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20101112012315.GE17097@core.coreip.homeip.net> Sender: linux-kernel-owner@vger.kernel.org To: Dmitry Torokhov Cc: Boaz Harrosh , Greg KH , linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org, scst-devel , James Bottomley , Andrew Morton , FUJITA Tomonori , Mike Christie , Vu Pham , Bart Van Assche , James Smart , Joe Eykholt , Andy Yan , Chetan Loke , Hannes Reinecke , Richard Sharpe , Daniel Henrique Debonzi List-Id: linux-scsi@vger.kernel.org Dmitry Torokhov, on 11/12/2010 04:23 AM wrote: >>> This put might not be the last put on the object, IOs in flight >>> and/or open files might have extra reference on the object. >>> We release our initial ref, and below wait for all operations >>> to complete. (Is there a matter of timeout like files not closing?) >> >> This is the last internal put. All other references are from outsiders. >> So, we are waiting for all them to put before we go on. >> > > The question is why do you need to wait here? I presume it is module > unloading path, but then it is quite bad - you can easily wedge your > subsystem if you make something to take a reference to your kobject > while module is ytying to be unloaded. Back when sysfs attributes tied > kobjects the easiest thing was to do: > > rmmod < / sys/devices/..../attribute > > If you are done with the kobject - just proceed with what you were doing > and let it die its own peaceful death some time later. You just need to > make sure release code sticks around to free it and your subsystem core > can be tasked with this. Use module counter to prevent unloading of the > subsystem core until all kobjects belonging to the subsystem are > destroyed. This is a very good question. During implementation I spent a lot of time working on it. In fact, the first implementation was asynchronous similarly as you proposing, i.e. it just proceed with what you were doing and let it die its own peaceful death some time later. But soon the implementation was becoming so complicated, so it started getting out of control. For instance, some of the tasks to solve with this approach were: 1. What to do if another SCST object is being created with the same name as supposed to be deleted, but not completely dead yet? 2. What to do if a dieing object is found on some list and reference for is supposed to be taken? If the object deleted from the list before it marked dieing, i.e. the latest internal put() done, it made additional problems during deleting it after the latest external put done. ... So, I decided to reimplement it to be completely synchronous. SYSFS authors did really great job and thanks to the excellent internal SYSFS design and implementation it is absolutely safe. See: [root@tgt ~]# modprobe scst [root@tgt ~]# cd /sys/kernel/scst_tgt/ [root@tgt scst_tgt]# ls -l total 0 drwxr-xr-x 4 root root 0 Nov 13 21:31 devices drwxr-xr-x 2 root root 0 Nov 13 21:31 handlers -r--r--r-- 1 root root 4096 Nov 13 21:30 last_sysfs_mgmt_res -rw-r--r-- 1 root root 4096 Nov 13 21:30 setup_id drwxr-xr-x 5 root root 0 Nov 13 21:31 sgv drwxr-xr-x 2 root root 0 Nov 13 21:31 targets -rw-r--r-- 1 root root 4096 Nov 13 21:30 threads -rw-r--r-- 1 root root 4096 Nov 13 21:30 trace_level -r--r--r-- 1 root root 4096 Nov 13 21:30 version [root@tgt scst_tgt]# cat version 2.1.0-pre1 EXTRACHECKS DEBUG [root@tgt scst_tgt]# rmmod scst