Re: [patch 13/13] s390: dasd device identifiers.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 2006-04-24 at 16:49 -0700, Andrew Morton wrote:
> Martin Schwidefsky <[email protected]> wrote:
> >
> > @@ -45,6 +45,7 @@ struct dasd_devmap {
> >          unsigned int devindex;
> >          unsigned short features;
> >  	struct dasd_device *device;
> > +	struct dasd_uid uid;
> >  };
> 
> Someone's missing a TAB key.

I have to find some script that checks for the damned tabs.

> > +static ssize_t
> > +dasd_alias_show(struct device *dev, struct device_attribute *attr, char *buf)
> > +{
> > +	struct dasd_devmap *devmap;
> > +	int alias;
> > +
> > +	devmap = dasd_find_busid(dev->bus_id);
> > +	spin_lock(&dasd_devmap_lock);
> > +	if (!IS_ERR(devmap))
> > +		alias = devmap->uid.alias;
> > +	else
> > +		alias = 0;
> > +	spin_unlock(&dasd_devmap_lock);
> > +
> > +	return sprintf(buf, alias ? "1\n" : "0\n");
> > +}
> 
> The locking is suspicious.  We take a spinlock just for a single read?

We could remove the locking for single read/writes, devmap structures
are never deleted so it is safe to access a single value after the
dasd_find_busid. But it is not so easy for the vendor and serial string,
these are not single values so we need some kind of locking. To keep
things symmetrical the lock is used for alias as well. There is a bug
though, only the pointer to the vendor/serial string is read under the
lock, the copy is done outside the lock. If that races with a
dasd_set_uid call funny things might happen. The copy needs to be done
inside the lock. Horst ?

-- 
blue skies,
  Martin.

Martin Schwidefsky
Linux for zSeries Development & Services
IBM Deutschland Entwicklung GmbH

"Reality continues to ruin my life." - Calvin.


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux