Messages in this thread | | | From | Kees Cook <> | Date | Thu, 8 Sep 2016 14:33:52 -0700 | Subject | Re: [PATCH v2 3/5] pstore: support multiple pmsg instances |
| |
On Sun, Jul 24, 2016 at 8:56 PM, Nobuhiro Iwamatsu <nobuhiro.iwamatsu.kw@hitachi.com> wrote: > From: Hiraku Toyooka <hiraku.toyooka.gu@hitachi.com> > > This enables pmsg to deal with multiple instances. One possible > use is content priority control on limited persistent store space. By > using different buffers, we can prevent important messages from being > overwritten by less important messages. > > When pstore backend module specifies the number of instances (num_pmsg) > in pstore_info, multiple /dev/pmsg[ID] appear. (ID is an integer > starting at 0. It corresponds to minor number of the each char device.) > Writes to each /dev/pmsg[ID] are isolated each other. After reboot, the > contents are available in /sys/fs/pstore/pmsg-[backend]-[ID]. > > Signed-off-by: Hiraku Toyooka <hiraku.toyooka.gu@hitachi.com> > Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.kw@hitachi.com> > Cc: Mark Salyzyn <salyzyn@android.com> > Cc: Seiji Aguchi <seiji.aguchi.tr@hitachi.com> > --- > fs/pstore/pmsg.c | 20 ++++++++++++++++++-- > include/linux/pstore.h | 1 + > 2 files changed, 19 insertions(+), 2 deletions(-) > > diff --git a/fs/pstore/pmsg.c b/fs/pstore/pmsg.c > index 7de20cd..2e281ed 100644 > --- a/fs/pstore/pmsg.c > +++ b/fs/pstore/pmsg.c > @@ -52,8 +52,8 @@ static ssize_t write_pmsg(struct file *file, const char __user *buf, > vfree(buffer); > return -EFAULT; > } > - psinfo->write_buf(PSTORE_TYPE_PMSG, 0, &id, 0, buffer, 0, c, > - psinfo); > + psinfo->write_buf(PSTORE_TYPE_PMSG, 0, &id, > + iminor(file->f_inode), buffer, 0, c, psinfo); > > i += c; > } > @@ -85,6 +85,7 @@ static char *pmsg_devnode(struct device *dev, umode_t *mode) > void pstore_register_pmsg(void) > { > struct device *pmsg_device; > + int i = 0; > > pmsg_major = register_chrdev(0, PMSG_NAME, &pmsg_fops); > if (pmsg_major < 0) { > @@ -105,9 +106,24 @@ void pstore_register_pmsg(void) > pr_err("failed to create device\n"); > goto err_device; > } > + > + /* allocate additional /dev/pmsg[ID] */ > + for (i = 1; i < psinfo->num_pmsg; i++) { > + struct device *pmsg_device; > + > + pmsg_device = device_create(pmsg_class, NULL, > + MKDEV(pmsg_major, i), NULL, "%s%d", > + PMSG_NAME, i); > + if (IS_ERR(pmsg_device)) { > + pr_err("failed to create device\n"); > + goto err_device; > + } > + }
This should just be merged into the first device_create call (i.e. drop the first one, keep your loop, but have "i" start at 0). There's no reason that I can see to do it separately.
> return; > > err_device: > + for (i--; i >= 0; i--) > + device_destroy(pmsg_class, MKDEV(pmsg_major, i));
Was the device_destroy() for /dev/pmsg0 missing before? (i.e. does class_destroy() or unregister_chrdev() already clean up the devices?)
> class_destroy(pmsg_class); > err_class: > unregister_chrdev(pmsg_major, PMSG_NAME); > diff --git a/include/linux/pstore.h b/include/linux/pstore.h > index 831479f..b0c24cc 100644 > --- a/include/linux/pstore.h > +++ b/include/linux/pstore.h > @@ -54,6 +54,7 @@ struct pstore_info { > size_t bufsize; > struct mutex read_mutex; /* serialize open/read/close */ > int flags; > + unsigned int num_pmsg; > int (*open)(struct pstore_info *psi); > int (*close)(struct pstore_info *psi); > ssize_t (*read)(u64 *id, enum pstore_type_id *type, > -- > 2.8.1 > >
-Kees
-- Kees Cook Nexus Security
| |