All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andre Noll <maan@systemlinux.org>
To: Boaz Harrosh <bharrosh@panasas.com>
Cc: Andi Kleen <andi@firstfloor.org>,
	James Bottomley <James.Bottomley@HansenPartnership.com>,
	linux-scsi <linux-scsi@vger.kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	kernel-janitors@vger.kernel.org,
	Richard Knutsson <ricknu-0@student.ltu.se>
Subject: Re: [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl
Date: Thu, 10 Jan 2008 20:40:20 +0000	[thread overview]
Message-ID: <20080110204020.GC20152@skl-net.de> (raw)
In-Reply-To: <47867C7E.3000100@panasas.com>

[-- Attachment #1: Type: text/plain, Size: 1167 bytes --]

On 22:13, Boaz Harrosh wrote:

> All the scsi calls do not need any locks. The scsi LLDS never
> see these threads since commands are queued through the block
> layer.

That's what everybody believes, but nobody seems to know for sure.
Therefore I did what Andi suggested: Make a zero-semantics change
that moves the lock_kernel() to sg_ioctl() to make people aware of
the fact that this function runs under the BKL. At least the latter
has already succeeded.

> What's left is what you see, here in sg.c. you must have the best 
> knowledge about the possible races between ioctl and open/release
> and probe/remove. And all these put_user() copy_user() etc...
> Why don't you have a hard look and fix them properly.

Because that requires much more knowledge. Al is looking into this
which indicates that it is non-trivial issue. I am clearly not the
right person to decide this question.

> please don't *lock_kernel();* for scsi's sake.

The BKL was there all the time. My patch just made it more visable
to the scsi people by moving it into sg.c.

Andre
-- 
The only person who always got his work done by Friday was Robinson Crusoe

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Andre Noll <maan@systemlinux.org>
To: Boaz Harrosh <bharrosh@panasas.com>
Cc: Andi Kleen <andi@firstfloor.org>,
	James Bottomley <James.Bottomley@HansenPartnership.com>,
	linux-scsi <linux-scsi@vger.kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	kernel-janitors@vger.kernel.org,
	Richard Knutsson <ricknu-0@student.ltu.se>
Subject: Re: [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl
Date: Thu, 10 Jan 2008 21:40:20 +0100	[thread overview]
Message-ID: <20080110204020.GC20152@skl-net.de> (raw)
In-Reply-To: <47867C7E.3000100@panasas.com>

[-- Attachment #1: Type: text/plain, Size: 1167 bytes --]

On 22:13, Boaz Harrosh wrote:

> All the scsi calls do not need any locks. The scsi LLDS never
> see these threads since commands are queued through the block
> layer.

That's what everybody believes, but nobody seems to know for sure.
Therefore I did what Andi suggested: Make a zero-semantics change
that moves the lock_kernel() to sg_ioctl() to make people aware of
the fact that this function runs under the BKL. At least the latter
has already succeeded.

> What's left is what you see, here in sg.c. you must have the best 
> knowledge about the possible races between ioctl and open/release
> and probe/remove. And all these put_user() copy_user() etc...
> Why don't you have a hard look and fix them properly.

Because that requires much more knowledge. Al is looking into this
which indicates that it is non-trivial issue. I am clearly not the
right person to decide this question.

> please don't *lock_kernel();* for scsi's sake.

The BKL was there all the time. My patch just made it more visable
to the scsi people by moving it into sg.c.

Andre
-- 
The only person who always got his work done by Friday was Robinson Crusoe

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

  reply	other threads:[~2008-01-10 20:40 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-10 18:05 [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl Andre Noll
2008-01-10 18:05 ` Andre Noll
2008-01-10 18:54 ` [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to James Bottomley
2008-01-10 18:54   ` [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl James Bottomley
2008-01-10 18:59   ` Andi Kleen
2008-01-10 18:59     ` Andi Kleen
2008-01-10 19:03     ` Matthew Wilcox
2008-01-10 19:03       ` Matthew Wilcox
2008-01-10 19:21       ` Andi Kleen
2008-01-10 19:21         ` Andi Kleen
2008-01-10 19:03     ` [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to James Bottomley
2008-01-10 19:03       ` [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl James Bottomley
2008-01-10 19:32       ` Andi Kleen
2008-01-10 19:32         ` Andi Kleen
2008-01-10 19:33         ` Matthew Wilcox
2008-01-10 19:33           ` Matthew Wilcox
2008-01-10 19:38           ` Andi Kleen
2008-01-10 19:38             ` Andi Kleen
2008-01-10 19:07     ` Andre Noll
2008-01-10 19:07       ` Andre Noll
2008-01-10 19:29       ` Andi Kleen
2008-01-10 19:29         ` Andi Kleen
2008-01-10 19:45         ` Andre Noll
2008-01-10 19:45           ` Andre Noll
2008-01-10 20:09           ` [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to James Bottomley
2008-01-10 20:09             ` [patch 1/1] Switch ioctl functions of drivers/scsi/sg.c to unlocked_ioctl James Bottomley
2008-01-10 20:13           ` Boaz Harrosh
2008-01-10 20:13             ` Boaz Harrosh
2008-01-10 20:40             ` Andre Noll [this message]
2008-01-10 20:40               ` Andre Noll

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=20080110204020.GC20152@skl-net.de \
    --to=maan@systemlinux.org \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=andi@firstfloor.org \
    --cc=bharrosh@panasas.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=ricknu-0@student.ltu.se \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.