lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 12 Jan 2021 10:00:25 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Greg KH' <gregkh@...uxfoundation.org>,
        Matthew Wilcox <willy@...radead.org>
CC:     Christoph Hellwig <hch@....de>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>
Subject: RE: [PATCH] char_dev: replace cdev_map with an xarray

From: Greg KH
> Sent: 12 January 2021 09:35
> 
> On Mon, Jan 11, 2021 at 08:58:18PM +0000, Matthew Wilcox wrote:
> > On Mon, Jan 11, 2021 at 06:05:13PM +0100, Christoph Hellwig wrote:
> > > @@ -486,14 +491,22 @@ int cdev_add(struct cdev *p, dev_t dev, unsigned count)
> > >  	if (WARN_ON(dev == WHITEOUT_DEV))
> > >  		return -EBUSY;
> > >
> > > -	error = kobj_map(cdev_map, dev, count, NULL,
> > > -			 exact_match, exact_lock, p);
> > > -	if (error)
> > > -		return error;
> > > +	mutex_lock(&chrdevs_lock);
> > > +	for (i = 0; i < count; i++) {
> > > +		error = xa_insert(&cdev_map, dev + i, p, GFP_KERNEL);
> > > +		if (error)
> > > +			goto out_unwind;
> > > +	}
> > > +	mutex_unlock(&chrdevs_lock);
> >
> > Looking at some of the users ...
> >
> > #define BSG_MAX_DEVS            32768
> > ...
> >         ret = cdev_add(&bsg_cdev, MKDEV(bsg_major, 0), BSG_MAX_DEVS);
> >
> > So this is going to allocate 32768 entries; at 8 bytes each, that's 256kB.
> > With XArray overhead, it works out to 73 pages or 292kB.  While I don't
> > have bsg loaded on my laptop, I imagine a lot of machines do.
> >
> > drivers/net/tap.c:#define TAP_NUM_DEVS (1U << MINORBITS)
> > include/linux/kdev_t.h:#define MINORBITS        20
> > drivers/net/tap.c:      err = cdev_add(tap_cdev, *tap_major, TAP_NUM_DEVS);
> >
> > That's going to be even worse -- 8MB plus the overhead to be closer to 9MB.
> >
> > I think we do need to implement the 'store a range' option ;-(
> 
> Looks like it will be needed here.
> 
> Wow that's a lot of tap devices allocated all at once, odd...

Aren't there two distinct use cases that probably need treating
separately?

1) A driver that uses it's own major (private) major number.
2) Drivers that share a major number (eg serial ports).

In the first case you just want all minors to come to your driver.
In the second case different (ranges of) minors need to go to
different drivers.

Until the major number is actually shared only the upper limit
is an any way relevant.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Powered by blists - more mailing lists