* [PATCH] bcachefs: chardev: make bch_chardev_class constant
@ 2024-03-05 20:21 Ricardo B. Marliere
2024-03-06 1:23 ` Hongbo Li
0 siblings, 1 reply; 6+ messages in thread
From: Ricardo B. Marliere @ 2024-03-05 20:21 UTC (permalink / raw)
To: Kent Overstreet, Brian Foster
Cc: linux-bcachefs, linux-kernel, Greg Kroah-Hartman, Ricardo B. Marliere
Since commit 43a7206b0963 ("driver core: class: make class_register() take
a const *"), the driver core allows for struct class to be in read-only
memory, so move the bch_chardev_class structure to be declared at build
time placing it into read-only memory, instead of having to be dynamically
allocated at boot time.
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Ricardo B. Marliere <ricardo@marliere.net>
---
fs/bcachefs/chardev.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/fs/bcachefs/chardev.c b/fs/bcachefs/chardev.c
index 226b39c17667..af587453fd3d 100644
--- a/fs/bcachefs/chardev.c
+++ b/fs/bcachefs/chardev.c
@@ -940,7 +940,9 @@ static const struct file_operations bch_chardev_fops = {
};
static int bch_chardev_major;
-static struct class *bch_chardev_class;
+static const struct class bch_chardev_class = {
+ .name = "bcachefs",
+};
static struct device *bch_chardev;
void bch2_fs_chardev_exit(struct bch_fs *c)
@@ -957,7 +959,7 @@ int bch2_fs_chardev_init(struct bch_fs *c)
if (c->minor < 0)
return c->minor;
- c->chardev = device_create(bch_chardev_class, NULL,
+ c->chardev = device_create(&bch_chardev_class, NULL,
MKDEV(bch_chardev_major, c->minor), c,
"bcachefs%u-ctl", c->minor);
if (IS_ERR(c->chardev))
@@ -968,26 +970,25 @@ int bch2_fs_chardev_init(struct bch_fs *c)
void bch2_chardev_exit(void)
{
- if (!IS_ERR_OR_NULL(bch_chardev_class))
- device_destroy(bch_chardev_class,
- MKDEV(bch_chardev_major, U8_MAX));
- if (!IS_ERR_OR_NULL(bch_chardev_class))
- class_destroy(bch_chardev_class);
+ device_destroy(&bch_chardev_class, MKDEV(bch_chardev_major, U8_MAX));
+ class_unregister(&bch_chardev_class);
if (bch_chardev_major > 0)
unregister_chrdev(bch_chardev_major, "bcachefs");
}
int __init bch2_chardev_init(void)
{
+ int ret;
+
bch_chardev_major = register_chrdev(0, "bcachefs-ctl", &bch_chardev_fops);
if (bch_chardev_major < 0)
return bch_chardev_major;
- bch_chardev_class = class_create("bcachefs");
- if (IS_ERR(bch_chardev_class))
- return PTR_ERR(bch_chardev_class);
+ ret = class_register(&bch_chardev_class);
+ if (ret)
+ return ret;
- bch_chardev = device_create(bch_chardev_class, NULL,
+ bch_chardev = device_create(&bch_chardev_class, NULL,
MKDEV(bch_chardev_major, U8_MAX),
NULL, "bcachefs-ctl");
if (IS_ERR(bch_chardev))
---
base-commit: 90d35da658da8cff0d4ecbb5113f5fac9d00eb72
change-id: 20240305-bcachefs-27a4bb8b9f4f
Best regards,
--
Ricardo B. Marliere <ricardo@marliere.net>
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] bcachefs: chardev: make bch_chardev_class constant
2024-03-05 20:21 [PATCH] bcachefs: chardev: make bch_chardev_class constant Ricardo B. Marliere
@ 2024-03-06 1:23 ` Hongbo Li
2024-03-06 11:50 ` Ricardo B. Marliere
0 siblings, 1 reply; 6+ messages in thread
From: Hongbo Li @ 2024-03-06 1:23 UTC (permalink / raw)
To: Ricardo B. Marliere, Kent Overstreet, Brian Foster
Cc: linux-bcachefs, linux-kernel, Greg Kroah-Hartman
On 2024/3/6 4:21, Ricardo B. Marliere wrote:
> Since commit 43a7206b0963 ("driver core: class: make class_register() take
> a const *"), the driver core allows for struct class to be in read-only
> memory, so move the bch_chardev_class structure to be declared at build
> time placing it into read-only memory, instead of having to be dynamically
> allocated at boot time.
>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Ricardo B. Marliere <ricardo@marliere.net>
> ---
> fs/bcachefs/chardev.c | 23 ++++++++++++-----------
> 1 file changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/fs/bcachefs/chardev.c b/fs/bcachefs/chardev.c
> index 226b39c17667..af587453fd3d 100644
> --- a/fs/bcachefs/chardev.c
> +++ b/fs/bcachefs/chardev.c
> @@ -940,7 +940,9 @@ static const struct file_operations bch_chardev_fops = {
> };
>
> static int bch_chardev_major;
> -static struct class *bch_chardev_class;
> +static const struct class bch_chardev_class = {
> + .name = "bcachefs",
> +};
> static struct device *bch_chardev;
>
> void bch2_fs_chardev_exit(struct bch_fs *c)
> @@ -957,7 +959,7 @@ int bch2_fs_chardev_init(struct bch_fs *c)
> if (c->minor < 0)
> return c->minor;
>
> - c->chardev = device_create(bch_chardev_class, NULL,
> + c->chardev = device_create(&bch_chardev_class, NULL,
> MKDEV(bch_chardev_major, c->minor), c,
> "bcachefs%u-ctl", c->minor);
> if (IS_ERR(c->chardev))
> @@ -968,26 +970,25 @@ int bch2_fs_chardev_init(struct bch_fs *c)
>
> void bch2_chardev_exit(void)
> {
> - if (!IS_ERR_OR_NULL(bch_chardev_class))
> - device_destroy(bch_chardev_class,
> - MKDEV(bch_chardev_major, U8_MAX));
> - if (!IS_ERR_OR_NULL(bch_chardev_class))
> - class_destroy(bch_chardev_class);
> + device_destroy(&bch_chardev_class, MKDEV(bch_chardev_major, U8_MAX));
> + class_unregister(&bch_chardev_class);
> if (bch_chardev_major > 0)
> unregister_chrdev(bch_chardev_major, "bcachefs");
> }
>
> int __init bch2_chardev_init(void)
> {
> + int ret;
> +
> bch_chardev_major = register_chrdev(0, "bcachefs-ctl", &bch_chardev_fops);
> if (bch_chardev_major < 0)
> return bch_chardev_major;
>
> - bch_chardev_class = class_create("bcachefs");
> - if (IS_ERR(bch_chardev_class))
> - return PTR_ERR(bch_chardev_class);
> + ret = class_register(&bch_chardev_class);
> + if (ret)
Here, I think you should call class_unregister to relase resource which
allocated before. And the same thing shoud be done in other exception exit.
> + return ret;
>
> - bch_chardev = device_create(bch_chardev_class, NULL,
> + bch_chardev = device_create(&bch_chardev_class, NULL,
> MKDEV(bch_chardev_major, U8_MAX),
> NULL, "bcachefs-ctl");
> if (IS_ERR(bch_chardev))
like here..
>
> ---
> base-commit: 90d35da658da8cff0d4ecbb5113f5fac9d00eb72
> change-id: 20240305-bcachefs-27a4bb8b9f4f
>
> Best regards,
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bcachefs: chardev: make bch_chardev_class constant
2024-03-06 1:23 ` Hongbo Li
@ 2024-03-06 11:50 ` Ricardo B. Marliere
2024-03-06 13:01 ` Hongbo Li
2024-03-08 2:18 ` Kent Overstreet
0 siblings, 2 replies; 6+ messages in thread
From: Ricardo B. Marliere @ 2024-03-06 11:50 UTC (permalink / raw)
To: Hongbo Li
Cc: Kent Overstreet, Brian Foster, linux-bcachefs, linux-kernel,
Greg Kroah-Hartman
On 6 Mar 09:23, Hongbo Li wrote:
> On 2024/3/6 4:21, Ricardo B. Marliere wrote:
> > Since commit 43a7206b0963 ("driver core: class: make class_register() take
> > a const *"), the driver core allows for struct class to be in read-only
> > memory, so move the bch_chardev_class structure to be declared at build
> > time placing it into read-only memory, instead of having to be dynamically
> > allocated at boot time.
> >
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Signed-off-by: Ricardo B. Marliere <ricardo@marliere.net>
> > ---
> > fs/bcachefs/chardev.c | 23 ++++++++++++-----------
> > 1 file changed, 12 insertions(+), 11 deletions(-)
> >
> > diff --git a/fs/bcachefs/chardev.c b/fs/bcachefs/chardev.c
> > index 226b39c17667..af587453fd3d 100644
> > --- a/fs/bcachefs/chardev.c
> > +++ b/fs/bcachefs/chardev.c
> > @@ -940,7 +940,9 @@ static const struct file_operations bch_chardev_fops = {
> > };
> >
> > static int bch_chardev_major;
> > -static struct class *bch_chardev_class;
> > +static const struct class bch_chardev_class = {
> > + .name = "bcachefs",
> > +};
> > static struct device *bch_chardev;
> >
> > void bch2_fs_chardev_exit(struct bch_fs *c)
> > @@ -957,7 +959,7 @@ int bch2_fs_chardev_init(struct bch_fs *c)
> > if (c->minor < 0)
> > return c->minor;
> >
> > - c->chardev = device_create(bch_chardev_class, NULL,
> > + c->chardev = device_create(&bch_chardev_class, NULL,
> > MKDEV(bch_chardev_major, c->minor), c,
> > "bcachefs%u-ctl", c->minor);
> > if (IS_ERR(c->chardev))
> > @@ -968,26 +970,25 @@ int bch2_fs_chardev_init(struct bch_fs *c)
> >
> > void bch2_chardev_exit(void)
> > {
> > - if (!IS_ERR_OR_NULL(bch_chardev_class))
> > - device_destroy(bch_chardev_class,
> > - MKDEV(bch_chardev_major, U8_MAX));
> > - if (!IS_ERR_OR_NULL(bch_chardev_class))
> > - class_destroy(bch_chardev_class);
> > + device_destroy(&bch_chardev_class, MKDEV(bch_chardev_major, U8_MAX));
> > + class_unregister(&bch_chardev_class);
> > if (bch_chardev_major > 0)
> > unregister_chrdev(bch_chardev_major, "bcachefs");
> > }
> >
> > int __init bch2_chardev_init(void)
> > {
> > + int ret;
> > +
> > bch_chardev_major = register_chrdev(0, "bcachefs-ctl", &bch_chardev_fops);
> > if (bch_chardev_major < 0)
> > return bch_chardev_major;
> >
> > - bch_chardev_class = class_create("bcachefs");
> > - if (IS_ERR(bch_chardev_class))
> > - return PTR_ERR(bch_chardev_class);
> > + ret = class_register(&bch_chardev_class);
> > + if (ret)
> Here, I think you should call class_unregister to relase resource which
> allocated before. And the same thing shoud be done in other exception exit.
Hi Hongbo,
Thank you for the feedback. Did you mean that bch_chardev_major should
be unregistered if the class_register() call fails? Because if it does,
there is no need to call class_unregister().
> > + return ret;
> >
> > - bch_chardev = device_create(bch_chardev_class, NULL,
> > + bch_chardev = device_create(&bch_chardev_class, NULL,
> > MKDEV(bch_chardev_major, U8_MAX),
> > NULL, "bcachefs-ctl");
> > if (IS_ERR(bch_chardev))
> like here..
Can you please elaborate?
Best regards,
- Ricardo.
> >
> > ---
> > base-commit: 90d35da658da8cff0d4ecbb5113f5fac9d00eb72
> > change-id: 20240305-bcachefs-27a4bb8b9f4f
> >
> > Best regards,
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bcachefs: chardev: make bch_chardev_class constant
2024-03-06 11:50 ` Ricardo B. Marliere
@ 2024-03-06 13:01 ` Hongbo Li
2024-03-08 2:18 ` Kent Overstreet
1 sibling, 0 replies; 6+ messages in thread
From: Hongbo Li @ 2024-03-06 13:01 UTC (permalink / raw)
To: Ricardo B. Marliere
Cc: Kent Overstreet, Brian Foster, linux-bcachefs, linux-kernel,
Greg Kroah-Hartman
On 2024/3/6 19:50, Ricardo B. Marliere wrote:
> On 6 Mar 09:23, Hongbo Li wrote:
>> On 2024/3/6 4:21, Ricardo B. Marliere wrote:
>>> Since commit 43a7206b0963 ("driver core: class: make class_register() take
>>> a const *"), the driver core allows for struct class to be in read-only
>>> memory, so move the bch_chardev_class structure to be declared at build
>>> time placing it into read-only memory, instead of having to be dynamically
>>> allocated at boot time.
>>>
>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> Signed-off-by: Ricardo B. Marliere <ricardo@marliere.net>
>>> ---
>>> fs/bcachefs/chardev.c | 23 ++++++++++++-----------
>>> 1 file changed, 12 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/fs/bcachefs/chardev.c b/fs/bcachefs/chardev.c
>>> index 226b39c17667..af587453fd3d 100644
>>> --- a/fs/bcachefs/chardev.c
>>> +++ b/fs/bcachefs/chardev.c
>>> @@ -940,7 +940,9 @@ static const struct file_operations bch_chardev_fops = {
>>> };
>>>
>>> static int bch_chardev_major;
>>> -static struct class *bch_chardev_class;
>>> +static const struct class bch_chardev_class = {
>>> + .name = "bcachefs",
>>> +};
>>> static struct device *bch_chardev;
>>>
>>> void bch2_fs_chardev_exit(struct bch_fs *c)
>>> @@ -957,7 +959,7 @@ int bch2_fs_chardev_init(struct bch_fs *c)
>>> if (c->minor < 0)
>>> return c->minor;
>>>
>>> - c->chardev = device_create(bch_chardev_class, NULL,
>>> + c->chardev = device_create(&bch_chardev_class, NULL,
>>> MKDEV(bch_chardev_major, c->minor), c,
>>> "bcachefs%u-ctl", c->minor);
>>> if (IS_ERR(c->chardev))
>>> @@ -968,26 +970,25 @@ int bch2_fs_chardev_init(struct bch_fs *c)
>>>
>>> void bch2_chardev_exit(void)
>>> {
>>> - if (!IS_ERR_OR_NULL(bch_chardev_class))
>>> - device_destroy(bch_chardev_class,
>>> - MKDEV(bch_chardev_major, U8_MAX));
>>> - if (!IS_ERR_OR_NULL(bch_chardev_class))
>>> - class_destroy(bch_chardev_class);
>>> + device_destroy(&bch_chardev_class, MKDEV(bch_chardev_major, U8_MAX));
>>> + class_unregister(&bch_chardev_class);
>>> if (bch_chardev_major > 0)
>>> unregister_chrdev(bch_chardev_major, "bcachefs");
>>> }
>>>
>>> int __init bch2_chardev_init(void)
>>> {
>>> + int ret;
>>> +
>>> bch_chardev_major = register_chrdev(0, "bcachefs-ctl", &bch_chardev_fops);
>>> if (bch_chardev_major < 0)
>>> return bch_chardev_major;
>>>
>>> - bch_chardev_class = class_create("bcachefs");
>>> - if (IS_ERR(bch_chardev_class))
>>> - return PTR_ERR(bch_chardev_class);
>>> + ret = class_register(&bch_chardev_class);
>>> + if (ret)
>> Here, I think you should call class_unregister to relase resource which
>> allocated before. And the same thing shoud be done in other exception exit.
>
> Hi Hongbo,
>
> Thank you for the feedback. Did you mean that bch_chardev_major should
> be unregistered if the class_register() call fails? Because if it does,
> there is no need to call class_unregister().
>
>>> + return ret;
>>>
>>> - bch_chardev = device_create(bch_chardev_class, NULL,
>>> + bch_chardev = device_create(&bch_chardev_class, NULL,
>>> MKDEV(bch_chardev_major, U8_MAX),
>>> NULL, "bcachefs-ctl");
>>> if (IS_ERR(bch_chardev))
>> like here..
>
> Can you please elaborate?
>
Yes, this I mean. The example like this:
```c
major = register_chrdev(0, xx, xx);
class = class_create("bcachefs");
if (IS_ERR(class))
goto out_chrdev:
...
out_class:
class_unregister(xxx);
out_chrdev:
unregister_chrdev(xx, xx);
...
```
Although this was not part of your original intention, it is indeed a flaw.
> Best regards,
> - Ricardo.
>
>
>>>
>>> ---
>>> base-commit: 90d35da658da8cff0d4ecbb5113f5fac9d00eb72
>>> change-id: 20240305-bcachefs-27a4bb8b9f4f
>>>
>>> Best regards,
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bcachefs: chardev: make bch_chardev_class constant
2024-03-06 11:50 ` Ricardo B. Marliere
2024-03-06 13:01 ` Hongbo Li
@ 2024-03-08 2:18 ` Kent Overstreet
2024-03-08 12:14 ` Ricardo B. Marliere
1 sibling, 1 reply; 6+ messages in thread
From: Kent Overstreet @ 2024-03-08 2:18 UTC (permalink / raw)
To: Ricardo B. Marliere
Cc: Hongbo Li, Brian Foster, linux-bcachefs, linux-kernel,
Greg Kroah-Hartman
On Wed, Mar 06, 2024 at 08:50:04AM -0300, Ricardo B. Marliere wrote:
> On 6 Mar 09:23, Hongbo Li wrote:
> > On 2024/3/6 4:21, Ricardo B. Marliere wrote:
> > > Since commit 43a7206b0963 ("driver core: class: make class_register() take
> > > a const *"), the driver core allows for struct class to be in read-only
> > > memory, so move the bch_chardev_class structure to be declared at build
> > > time placing it into read-only memory, instead of having to be dynamically
> > > allocated at boot time.
> > >
> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Signed-off-by: Ricardo B. Marliere <ricardo@marliere.net>
> > > ---
> > > fs/bcachefs/chardev.c | 23 ++++++++++++-----------
> > > 1 file changed, 12 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/fs/bcachefs/chardev.c b/fs/bcachefs/chardev.c
> > > index 226b39c17667..af587453fd3d 100644
> > > --- a/fs/bcachefs/chardev.c
> > > +++ b/fs/bcachefs/chardev.c
> > > @@ -940,7 +940,9 @@ static const struct file_operations bch_chardev_fops = {
> > > };
> > >
> > > static int bch_chardev_major;
> > > -static struct class *bch_chardev_class;
> > > +static const struct class bch_chardev_class = {
> > > + .name = "bcachefs",
> > > +};
> > > static struct device *bch_chardev;
> > >
> > > void bch2_fs_chardev_exit(struct bch_fs *c)
> > > @@ -957,7 +959,7 @@ int bch2_fs_chardev_init(struct bch_fs *c)
> > > if (c->minor < 0)
> > > return c->minor;
> > >
> > > - c->chardev = device_create(bch_chardev_class, NULL,
> > > + c->chardev = device_create(&bch_chardev_class, NULL,
> > > MKDEV(bch_chardev_major, c->minor), c,
> > > "bcachefs%u-ctl", c->minor);
> > > if (IS_ERR(c->chardev))
> > > @@ -968,26 +970,25 @@ int bch2_fs_chardev_init(struct bch_fs *c)
> > >
> > > void bch2_chardev_exit(void)
> > > {
> > > - if (!IS_ERR_OR_NULL(bch_chardev_class))
> > > - device_destroy(bch_chardev_class,
> > > - MKDEV(bch_chardev_major, U8_MAX));
> > > - if (!IS_ERR_OR_NULL(bch_chardev_class))
> > > - class_destroy(bch_chardev_class);
> > > + device_destroy(&bch_chardev_class, MKDEV(bch_chardev_major, U8_MAX));
> > > + class_unregister(&bch_chardev_class);
> > > if (bch_chardev_major > 0)
> > > unregister_chrdev(bch_chardev_major, "bcachefs");
> > > }
> > >
> > > int __init bch2_chardev_init(void)
> > > {
> > > + int ret;
> > > +
> > > bch_chardev_major = register_chrdev(0, "bcachefs-ctl", &bch_chardev_fops);
> > > if (bch_chardev_major < 0)
> > > return bch_chardev_major;
> > >
> > > - bch_chardev_class = class_create("bcachefs");
> > > - if (IS_ERR(bch_chardev_class))
> > > - return PTR_ERR(bch_chardev_class);
> > > + ret = class_register(&bch_chardev_class);
> > > + if (ret)
> > Here, I think you should call class_unregister to relase resource which
> > allocated before. And the same thing shoud be done in other exception exit.
>
> Hi Hongbo,
>
> Thank you for the feedback. Did you mean that bch_chardev_major should
> be unregistered if the class_register() call fails? Because if it does,
> there is no need to call class_unregister().
Hongbo is correct, if bch2_chardev_init() fails it needs to clean up
anything it did.
>
> > > + return ret;
> > >
> > > - bch_chardev = device_create(bch_chardev_class, NULL,
> > > + bch_chardev = device_create(&bch_chardev_class, NULL,
> > > MKDEV(bch_chardev_major, U8_MAX),
> > > NULL, "bcachefs-ctl");
> > > if (IS_ERR(bch_chardev))
> > like here..
>
> Can you please elaborate?
>
> Best regards,
> - Ricardo.
>
>
> > >
> > > ---
> > > base-commit: 90d35da658da8cff0d4ecbb5113f5fac9d00eb72
> > > change-id: 20240305-bcachefs-27a4bb8b9f4f
> > >
> > > Best regards,
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bcachefs: chardev: make bch_chardev_class constant
2024-03-08 2:18 ` Kent Overstreet
@ 2024-03-08 12:14 ` Ricardo B. Marliere
0 siblings, 0 replies; 6+ messages in thread
From: Ricardo B. Marliere @ 2024-03-08 12:14 UTC (permalink / raw)
To: Kent Overstreet
Cc: Hongbo Li, Brian Foster, linux-bcachefs, linux-kernel,
Greg Kroah-Hartman
On 7 Mar 21:18, Kent Overstreet wrote:
> On Wed, Mar 06, 2024 at 08:50:04AM -0300, Ricardo B. Marliere wrote:
> > On 6 Mar 09:23, Hongbo Li wrote:
> > > On 2024/3/6 4:21, Ricardo B. Marliere wrote:
> > > > Since commit 43a7206b0963 ("driver core: class: make class_register() take
> > > > a const *"), the driver core allows for struct class to be in read-only
> > > > memory, so move the bch_chardev_class structure to be declared at build
> > > > time placing it into read-only memory, instead of having to be dynamically
> > > > allocated at boot time.
> > > >
> > > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > Signed-off-by: Ricardo B. Marliere <ricardo@marliere.net>
> > > > ---
> > > > fs/bcachefs/chardev.c | 23 ++++++++++++-----------
> > > > 1 file changed, 12 insertions(+), 11 deletions(-)
> > > >
> > > > diff --git a/fs/bcachefs/chardev.c b/fs/bcachefs/chardev.c
> > > > index 226b39c17667..af587453fd3d 100644
> > > > --- a/fs/bcachefs/chardev.c
> > > > +++ b/fs/bcachefs/chardev.c
> > > > @@ -940,7 +940,9 @@ static const struct file_operations bch_chardev_fops = {
> > > > };
> > > >
> > > > static int bch_chardev_major;
> > > > -static struct class *bch_chardev_class;
> > > > +static const struct class bch_chardev_class = {
> > > > + .name = "bcachefs",
> > > > +};
> > > > static struct device *bch_chardev;
> > > >
> > > > void bch2_fs_chardev_exit(struct bch_fs *c)
> > > > @@ -957,7 +959,7 @@ int bch2_fs_chardev_init(struct bch_fs *c)
> > > > if (c->minor < 0)
> > > > return c->minor;
> > > >
> > > > - c->chardev = device_create(bch_chardev_class, NULL,
> > > > + c->chardev = device_create(&bch_chardev_class, NULL,
> > > > MKDEV(bch_chardev_major, c->minor), c,
> > > > "bcachefs%u-ctl", c->minor);
> > > > if (IS_ERR(c->chardev))
> > > > @@ -968,26 +970,25 @@ int bch2_fs_chardev_init(struct bch_fs *c)
> > > >
> > > > void bch2_chardev_exit(void)
> > > > {
> > > > - if (!IS_ERR_OR_NULL(bch_chardev_class))
> > > > - device_destroy(bch_chardev_class,
> > > > - MKDEV(bch_chardev_major, U8_MAX));
> > > > - if (!IS_ERR_OR_NULL(bch_chardev_class))
> > > > - class_destroy(bch_chardev_class);
> > > > + device_destroy(&bch_chardev_class, MKDEV(bch_chardev_major, U8_MAX));
> > > > + class_unregister(&bch_chardev_class);
> > > > if (bch_chardev_major > 0)
> > > > unregister_chrdev(bch_chardev_major, "bcachefs");
> > > > }
> > > >
> > > > int __init bch2_chardev_init(void)
> > > > {
> > > > + int ret;
> > > > +
> > > > bch_chardev_major = register_chrdev(0, "bcachefs-ctl", &bch_chardev_fops);
> > > > if (bch_chardev_major < 0)
> > > > return bch_chardev_major;
> > > >
> > > > - bch_chardev_class = class_create("bcachefs");
> > > > - if (IS_ERR(bch_chardev_class))
> > > > - return PTR_ERR(bch_chardev_class);
> > > > + ret = class_register(&bch_chardev_class);
> > > > + if (ret)
> > > Here, I think you should call class_unregister to relase resource which
> > > allocated before. And the same thing shoud be done in other exception exit.
> >
> > Hi Hongbo,
> >
> > Thank you for the feedback. Did you mean that bch_chardev_major should
> > be unregistered if the class_register() call fails? Because if it does,
> > there is no need to call class_unregister().
>
> Hongbo is correct, if bch2_chardev_init() fails it needs to clean up
> anything it did.
Hi Kent,
I agree. I sent a v2 but let me know if you want that to be in a
separate commit.
Best regards,
- Ricardo.
>
> >
> > > > + return ret;
> > > >
> > > > - bch_chardev = device_create(bch_chardev_class, NULL,
> > > > + bch_chardev = device_create(&bch_chardev_class, NULL,
> > > > MKDEV(bch_chardev_major, U8_MAX),
> > > > NULL, "bcachefs-ctl");
> > > > if (IS_ERR(bch_chardev))
> > > like here..
> >
> > Can you please elaborate?
> >
> > Best regards,
> > - Ricardo.
> >
> >
> > > >
> > > > ---
> > > > base-commit: 90d35da658da8cff0d4ecbb5113f5fac9d00eb72
> > > > change-id: 20240305-bcachefs-27a4bb8b9f4f
> > > >
> > > > Best regards,
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-03-08 12:14 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-05 20:21 [PATCH] bcachefs: chardev: make bch_chardev_class constant Ricardo B. Marliere
2024-03-06 1:23 ` Hongbo Li
2024-03-06 11:50 ` Ricardo B. Marliere
2024-03-06 13:01 ` Hongbo Li
2024-03-08 2:18 ` Kent Overstreet
2024-03-08 12:14 ` Ricardo B. Marliere
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.