Mike Snitzer <snitzer@xxxxxxxxxx> wrote:
> On Tue, May 04 2010 at 12:01am -0400,
> Mike Anderson <andmike@xxxxxxxxxxxxxxxxxx> wrote:
>
> > +++ b/drivers/md/dm-mpath.c
> > @@ -82,6 +82,7 @@ struct multipath {
> > unsigned saved_queue_if_no_path;/* Saved state during suspension */
> > unsigned pg_init_retries; /* Number of times to retry pg_init */
> > unsigned pg_init_count; /* Number of times pg_init called */
> > + unsigned long features;
>
> Why not use uint64_t?
I was just following dm.c mapped_device flags as an example, but could be
changed.
>
> > struct work_struct process_queued_ios;
> > struct list_head queued_ios;
> > @@ -118,6 +119,15 @@ static void trigger_event(struct work_struct *work);
> > static void activate_path(struct work_struct *work);
> > static void deactivate_path(struct work_struct *work);
> >
> > +static int multipath_test_feature(struct multipath *m, unsigned feature)
> > +{
> > + return test_bit(feature, &m->features);
> > +}
> > +
> > +static void multipath_set_feature(struct multipath *m, unsigned feature)
> > +{
> > + set_bit(feature, &m->features);
> > +}
>
> You're using 'unsigned long' for features yet these wrapper functions
> take 'unsigned'. unsigned allows you to use {test,set}_bit but in the
> end we have fewer flags to work with...
>
> Granted you're introducing the very first flag but... ;)
>
As I indicated above I used dm.c bit setting as an example. The feature
bit nr plus the {test,set}_bit appeared like a good start but as you said
I was just introducing the first flag. I guess it depends on how many
features we think we might need.
-andmike
--
Michael Anderson
andmike@xxxxxxxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
