Re: [PATCH 1/2] thread_options: Add cgroup_use_bfq

[Andreas Herrmann <aherrmann@suse.com>]

On Fri, Aug 28, 2020 at 05:52:11PM -0600, Jens Axboe wrote:
> On 8/28/20 12:36 PM, Elliott, Robert (Servers) wrote:
> > 
> > 
> >> -----Original Message-----
> >> From: fio-owner@vger.kernel.org <fio-owner@vger.kernel.org> On Behalf
> >> Of Andreas Herrmann
> >> Sent: Friday, August 28, 2020 5:05 AM
> >> To: Jens Axboe <axboe@kernel.dk>
> >> Cc: fio@vger.kernel.org
> >> Subject: [PATCH 1/2] thread_options: Add cgroup_use_bfq
> >>
> > ...
> >> @@ -593,11 +594,13 @@ struct thread_options_pack {
> >>  	uint8_t profile[FIO_TOP_STR_MAX];
> >>
> >>  	/*
> >> -	 * blkio cgroup support
> >> +	 * (blk)io cgroup support
> >>  	 */
> >>  	uint8_t cgroup[FIO_TOP_STR_MAX];
> >>  	uint32_t cgroup_weight;
> >>  	uint32_t cgroup_nodelete;
> >> +	uint32_t cgroup_use_bfq;
> >> +	uint32_t padding;       /* remove when possible to maintain alignment */
> >>
> >>  	uint32_t uid;
> >>  	uint32_t gid;
> > 
> > Since the structure already has three pad fields:
> >         uint32_t pad;
> > 
> >         uint64_t size;
> > 	...
> >         uint32_t pad2;
> >         uint64_t rand_seed;
> > 	...
> >         uint32_t pad3;
> >         fio_fp64_t percentile_list[FIO_IO_U_LIST_MAX_LEN];
> > 
> > pad4 would seem like the natural name for another padding field, and
> > the comment doesn't seem necessary.
> 
> Also worth checking if you can actually remove padding when updating it,
> rather than adding more.

I think I can't remove padding due to
fio_fp64_t latency_percentile;
later in this struct.

> BTW, this is also missing a FIO_SERVER_VER increment, that's needed
> whenever thread_options are changed.

I overlooked that. I'll adapt my change.

-- 
Regards,
Andreas

SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer (HRB 36809, AG Nürnberg)