Re: [PATCH v3 1/3] lib: bitmap: introduce bitmap_print_to_buf

[Greg KH]

On Thu, Jun 03, 2021 at 01:37:09PM +0200, Greg KH wrote:
> On Thu, Jun 03, 2021 at 07:21:30PM +0800, tiantao (H) wrote:
> > 
> > 在 2021/6/3 18:49, Greg KH 写道:
> > > On Thu, Jun 03, 2021 at 06:33:25PM +0800, tiantao (H) wrote:
> > > > 在 2021/6/3 17:50, Andy Shevchenko 写道:
> > > > > On Thu, Jun 03, 2021 at 05:22:40PM +0800, Tian Tao wrote:
> > > > > > New API bitmap_print_to_buf() with bin_attribute to avoid maskp
> > > > > > exceeding PAGE_SIZE. bitmap_print_to_pagebuf() is a special case
> > > > > > of bitmap_print_to_buf(), so in bitmap_print_to_pagebuf() call
> > > > > > bitmap_print_to_buf().
> > > > > ...
> > > > > 
> > > > > >    /**
> > > > > > + * bitmap_print_to_buf - convert bitmap to list or hex format ASCII string
> > > > > > + * @list: indicates whether the bitmap must be list
> > > > > > + * @buf: the kernel space buffer to read to
> > > > > > + * @maskp: pointer to bitmap to convert
> > > > > > + * @nmaskbits: size of bitmap, in bits
> > > > > > + * @off: offset in data buffer below
> > > > > > + * @count: the maximum number of bytes to print
> > > > > > + *
> > > > > > + * The role of bitmap_print_to_buf() and bitmap_print_to_pagebuf() is
> > > > > > + * the same, the difference is that buf of bitmap_print_to_buf()
> > > > > > + * can be more than one pagesize.
> > > > > > + */
> > > > > > +int bitmap_print_to_buf(bool list, char *buf, const unsigned long *maskp,
> > > > > > +			int nmaskbits, loff_t off, size_t count)
> > > > > > +{
> > > > > > +	const char *fmt = list ? "%*pbl\n" : "%*pb\n";
> > > > > > +	ssize_t size;
> > > > > > +	void *data;
> > > > > > +
> > > > > > +	if (off == LLONG_MAX && count == PAGE_SIZE - offset_in_page(buf))
> > > > > > +		return scnprintf(buf, count, fmt, nmaskbits, maskp);
> > > > > > +
> > > > > > +	data = kasprintf(GFP_KERNEL, fmt, nmaskbits, maskp);
> > > > > > +	if (!data)
> > > > > > +		return -ENOMEM;
> > > > > > +
> > > > > > +	size = memory_read_from_buffer(buf, count, &off, data, strlen(data) + 1);
> > > > > Are you sure you have put parameters in the correct order?
> > > > yes, I already test it.
> > > Great, can you add the test to the patch series as well so that we can
> > > make sure it does not break in the future?
> > How do I do this?  Do I need to provide a kselftest ?
> 
> That would be wonderful, great idea!

Or, as this is an internal kernel api, using kunit might be easier.

Obviously you tested this somehow, so just take advantage of the code
you wrote for that.

thanks,

greg k-h