All of lore.kernel.org
 help / color / mirror / Atom feed
From: Roland Dreier <rdreier@cisco.com>
To: "Bryan O'Sullivan" <bos@pathscale.com>
Cc: Greg KH <gregkh@suse.de>,
	rolandd@cisco.com, akpm@osdl.org, davem@davemloft.net,
	linux-kernel@vger.kernel.org, openib-general@openib.org
Subject: Re: [PATCH 9 of 20] ipath - char devices for diagnostics and lightweight subnet management
Date: Thu, 09 Mar 2006 21:55:02 -0800	[thread overview]
Message-ID: <ada7j72detl.fsf@cisco.com> (raw)
In-Reply-To: <1141965696.14517.4.camel@camp4.serpentine.com> (Bryan O'Sullivan's message of "Thu, 09 Mar 2006 20:41:36 -0800")

    Bryan> I *assumed* that there was something more that we would
    Bryan> need to do in order to support real hotplug of actual
    Bryan> physical cards, but now that I look more closely, it
    Bryan> doesn't appear that there is.  At least, there's nothing in
    Bryan> Documentation/pci.txt or LDD3 that indicates to me that we
    Bryan> ought to be doing more.

    Bryan> Am I missing something?

No, the only problems are with the way the various pieces of your
drivers refer to devices by index.  There are obvious races such as

 > +int __init ipath_verbs_init(void)
 > +{
 > +	int i;
 > +
 > +	number_of_devices = ipath_layer_get_num_of_dev();
 > +	i = number_of_devices * sizeof(struct ipath_ibdev *);
 > +	ipath_devices = kmalloc(i, GFP_ATOMIC);
 > +	if (ipath_devices == NULL)
 > +		return -ENOMEM;
 > +
 > +	for (i = 0; i < number_of_devices; i++) {
 > +		struct ipath_devdata *dd;
 > +		int ret = ipath_verbs_register(i, ipath_ib_piobufavail,
 > +					       ipath_ib_rcv, ipath_ib_timer,
 > +					       &dd);

suppose number_of_devices gets set to 5 but by the time you call
ipath_verbs_register(5,...), the 5th device has been unplugged?

Also you only do this when the module is loaded, so you won't handle
devices that are hot-plugged later.  And I don't see anything that
would handle hot unplug either.

Pretty much any use of ipath_max is probably broken by hot plug.

 - R.

  parent reply	other threads:[~2006-03-10  5:55 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <eac2ad3017b5f160d24c.1141922822@localhost.localdomain>
2006-03-09 23:20 ` [PATCH 9 of 20] ipath - char devices for diagnostics and lightweight subnet management Roland Dreier
2006-03-09 23:39   ` Bryan O'Sullivan
2006-03-09 23:47     ` Roland Dreier
2006-03-09 23:50       ` Bryan O'Sullivan
2006-03-09 23:52         ` Roland Dreier
2006-03-10 15:54     ` Michael S. Tsirkin
2006-03-10 16:05       ` Bryan O'Sullivan
2006-03-09 23:24 ` Roland Dreier
2006-03-09 23:49   ` Bryan O'Sullivan
2006-03-09 23:51     ` Roland Dreier
2006-03-09 23:26 ` Roland Dreier
2006-03-09 23:52   ` Bryan O'Sullivan
2006-03-10  0:00     ` Roland Dreier
2006-03-10  0:04       ` Bryan O'Sullivan
2006-03-10  0:45     ` Greg KH
2006-03-10  0:48       ` Bryan O'Sullivan
2006-03-10  1:04         ` Greg KH
2006-03-10  4:41           ` Bryan O'Sullivan
2006-03-10  5:48             ` Greg KH
2006-03-10 13:40               ` Bryan O'Sullivan
2006-03-10  5:55             ` Roland Dreier [this message]
2006-03-10 13:43               ` Bryan O'Sullivan
2006-03-10 16:58                 ` Greg KH
2006-03-10 17:05                   ` Bryan O'Sullivan
2006-03-10 17:08                 ` Roland Dreier
2006-03-10 17:32                   ` Bryan O'Sullivan
2006-03-10 22:20                     ` Roland Dreier
2006-03-10  0:35 [PATCH 0 of 20] [RFC] ipath driver - another round for review Bryan O'Sullivan
2006-03-10  0:35 ` [PATCH 9 of 20] ipath - char devices for diagnostics and lightweight subnet management Bryan O'Sullivan
2006-03-10  0:45   ` Roland Dreier
2006-03-10  0:47     ` Bryan O'Sullivan
2006-03-10  0:52       ` Roland Dreier

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=ada7j72detl.fsf@cisco.com \
    --to=rdreier@cisco.com \
    --cc=akpm@osdl.org \
    --cc=bos@pathscale.com \
    --cc=davem@davemloft.net \
    --cc=gregkh@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=openib-general@openib.org \
    --cc=rolandd@cisco.com \
    /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.