linux-kernel - RE: [PATCH] char_dev: replace cdev

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 linux-cve-announce 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]

Message-ID: <4e46c186b6a4493992d3a5cc15fb3bf3@AcuMS.aculab.com>
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)