public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Pete Zaitcev <zaitcev@redhat.com>
To: Douglas Gilbert <dougg@torque.net>
Cc: Pete Zaitcev <zaitcev@redhat.com>,
	linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: 2 questions about SCSI initialization
Date: Thu, 21 Mar 2002 19:19:38 -0500	[thread overview]
Message-ID: <20020321191938.B1054@devserv.devel.redhat.com> (raw)
In-Reply-To: <20020321000553.A6704@devserv.devel.redhat.com> <3C99E6C7.34F05AE7@torque.net>

> Date: Thu, 21 Mar 2002 08:57:27 -0500
> From: Douglas Gilbert <dougg@torque.net>

> > #1: Why does scsi_build_commandblocks() allocate memory with
> > GFP_ATOMIC? It's not called from an interrupt or from a swap I/O
> > path as far as I can see.
> 
> There has long been a preference in the scsi subsystem
> for using its own memory management ( scsi_malloc() )
> or the most conservative mm calls. GFP_ATOMIC may well
> be overkill in scsi_build_commandblocks(). However it
> might be wise to check that the calling context is indeed
> user_space since this can be called from all subsystems 
> that have a scsi pseudo device driver (e.g. ide-scsi, 
> usb-storage, 1394/sbp2, ...).

Thanks for the explanation. I've set out to prove that it is called
from a user context only. If that fails, I'll come up with something
else, perhaps in_interrupt() check, or an extra parameter.
 
> > #2: What does  if (GET_USE_COUNT(tpnt->module) != 0)  do in
> > scsi_unregister_device? The circomstances are truly bizzare:
> > a) the error code is NEVER used
> > b) it can be called either from module unload.
> > I would like to kill that check.
> 
> Another badly named function since it is unregistering
> in upper level driver (e.g. sd). That "if" is to check
> if there are open file descriptors (or some other
> reason **) on the driver in question. That seems to be
> a sensible check. Whether it can every happen in that
> context, I'm not sure.
> 
> ** The sg driver purposely holds its USE_COUNT > 0 even
> when all its file descriptors are closed iff there are
> outstanding commands for which the response has not
> yet arrived. [If this is not done, then a control-C on
> something like cdrecord followed by "rmmod sg" may
> cause an oops, especially during "fixating" mode.]

I think the above cannot apply, because sys_delete_module()
does this:

                spin_lock(&unload_lock);
                if (!__MOD_IN_USE(mod)) {
                        mod->flags |= MOD_DELETED;
                        spin_unlock(&unload_lock);
                        free_module(mod, 0);
                        error = 0;
                } else {
                        spin_unlock(&unload_lock);
                }

There's no way even to get near that check if the module
count is nonzero (well, minus "can_unload()", which is not 
used by sg, or anyone for that matter).

One more thing that was skipped in the header of my mail
was that the current code in scsi_register_device_module near
out_of_space flag simply DOES NOT WORK. There's an oops for
the show. It is possible to de-factor the scsi_unregister_device
into checking and non-checking version (perhaps one calling
the other), but why? So far it is proven that the check does
nothing (either it is not reached when files are open, or else
it is guaranteed to fail). This is why I suggest to remove
the check.

-- Pete

  parent reply	other threads:[~2002-03-22  0:22 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2002-03-21  5:05 2 questions about SCSI initialization Pete Zaitcev
2002-03-21 13:57 ` Douglas Gilbert
2002-03-21 14:32   ` Alan Cox
2002-03-22  0:19   ` Pete Zaitcev [this message]
2002-03-22  8:37   ` Pete Zaitcev
2002-03-21 22:26 ` Patrick Mansfield
2002-03-22  0:04   ` Pete Zaitcev
2002-03-22  1:27     ` Patrick Mansfield
2002-03-22  1:44       ` Pete Zaitcev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20020321191938.B1054@devserv.devel.redhat.com \
    --to=zaitcev@redhat.com \
    --cc=dougg@torque.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox