public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Rusty Russell <rusty@rustcorp.com.au>
To: Linus Torvalds <torvalds@transmeta.com>
Cc: Alexander Viro <viro@math.psu.edu>,
	Doug Ledford <dledford@redhat.com>,
	Linux Scsi Mailing List <linux-scsi@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: Why /dev/sdc1 doesn't show up...
Date: Wed, 20 Nov 2002 07:54:07 +1100	[thread overview]
Message-ID: <20021120072653.BC52C2C055@lists.samba.org> (raw)
In-Reply-To: Your message of "Mon, 18 Nov 2002 16:08:49 -0800." <Pine.LNX.4.44.0211181607120.13381-100000@home.transmeta.com>

In message <Pine.LNX.4.44.0211181607120.13381-100000@home.transmeta.com> you write:
> On Tue, 19 Nov 2002, Rusty Russell wrote:
> > 
> > See other posting.  This is a fundamental design decision, and it's
> > not changing.  Sorry.
> 
> Rusty, you take that approach, and the module code flies out of the 
> kernel. 

Linus,

	Sorry, you're right of course.  I should at least have done
something like the "unsafe" wedge which was used for the other
interfaces.  Under the circumstances, I agree with your fix.

> you seem to say that drivers should be broken een if they are perfectly 
> fine and do not have any races as is.

	Linus, I would like an answer: how does one register two /proc
entries?  The following has a race when modular:

	if (!create_proc_entry("foo"...))
		goto fail;
	if (!create_proc_entry("bar"...))
		goto fail_free_foo;
	return 0;

	I sympathize with Al's "we have worse problems, just BUG()
when it happens" approach, but I don't agree with Doug's "that's
clearly broken code!" assertion.  Al wants to split such code into:

	if (!reserve_proc_entry("foo"...))
		goto fail;
	if (!reserve_proc_entry("bar"...))
		goto fail_free_foo;

	/* Now we're safe! */
	activate_proc_entry("foo");
	activate_proc_entry("bar");
	return 0;

Note that this code change is *only* required because the code is
modular.  The code has works perfectly fine in the kernel, and has for
years, and very few people have ever noticed any problem with it.

Now, let's imagine one simple implementation of reserve_proc_entry and
activate_proc_entry: a proc entry gets a flag (call it "live") which
it can use to tell the difference between reserved and activated
entries, and it ignores !live ones.

Now, if you substitute the "live" flag in the proc entry for a "live"
flag in the proc->owner entry, you have the previous module code,
except:

1) You don't need to change all the modules to do two-stage init.
2) You don't need to change all the interfaces to two-stage init.
3) You *do* have problems with complex interfaces which need to do
   something more than just flip the flag to make it "live".

Now do you see why I like this scheme?  For most interfaces, it "just
works": I shall endevour to fix the remaining cases in a way which is
no less clear than current code, and if I fail, hey, we leave your
change in and we're no worse than 2.4.

> If so, then bye bye new module loader.

Fair enough.  Frankly, I just wanted to neaten the module code to
allow other improvements without breaking userspace every time.  But I
felt it wrong to re-implement the same module races: let this be a
valuable lesson to me not to bite off more than I can chew 8(

Thanks for your patience,
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

  reply	other threads:[~2002-11-20  7:19 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2002-11-17 19:52 Why /dev/sdc1 doesn't show up Doug Ledford
2002-11-17 20:01 ` Alexander Viro
2002-11-17 20:12   ` Doug Ledford
2002-11-17 20:16   ` Alexander Viro
2002-11-17 23:20 ` Andries Brouwer
2002-11-17 23:45   ` Doug Ledford
2002-11-18  8:52 ` Rusty Russell
2002-11-18  9:51   ` Alexander Viro
2002-11-18 23:49     ` Rusty Russell
2002-11-19  0:08       ` Linus Torvalds
2002-11-19 20:54         ` Rusty Russell [this message]
2002-11-20 15:45           ` Linus Torvalds
2002-11-24 22:30             ` Rusty Russell
2002-11-19  0:09       ` Doug Ledford
2002-11-19 20:58         ` Rusty Russell
2002-11-19  0:32       ` Alan Cox
2002-11-18 10:02   ` Roman Zippel
  -- strict thread matches above, loose matches on Subject: below --
2002-11-19  5:52 Rusty Russell
2002-11-19  7:12 ` Alexander Viro
2002-11-19 21:29   ` Rusty Russell
2002-11-19 22:33   ` Andries Brouwer
2002-11-19 16:06 ` Doug Ledford
2002-11-19 17:55 ` Jeff Garzik
2002-11-19 21:42   ` Rusty Russell
2002-11-20 23:41   ` john slee

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=20021120072653.BC52C2C055@lists.samba.org \
    --to=rusty@rustcorp.com.au \
    --cc=dledford@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=torvalds@transmeta.com \
    --cc=viro@math.psu.edu \
    /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