On Wed, Jan 07, 2009 at 06:42:37PM +0000, Chris Coulson wrote:
> On Wed, 2009-01-07 at 18:35 +0100, Jörg Sommer wrote:
> > Colin Watson schrieb am Wed 07. Jan, 12:18 (+0000):
> > > diff -Nru dpkg-1.14.22ubuntu1/utils/start-stop-daemon.c
> > > dpkg-1.14.22ubuntu2/utils/start-stop-daemon.c
> > > --- dpkg-1.14.22ubuntu1/utils/start-stop-daemon.c 2008-08-26
> > > 14:52:08.000000000 +0100
> > > +++ dpkg-1.14.22ubuntu2/utils/start-stop-daemon.c 2008-12-10
> > > 20:36:51.000000000 +0000
> > > @@ -1221,6 +1236,12 @@
> > > }
> > > }
> > >
> > > +static inline int
> > > +ioprio_set (int which, int who, int ioprio)
> > > +{
> > > + return syscall(SYS_ioprio_set, which, who, ioprio);
> >
> > Why don't you use ioprio_set() from the libc? Wouldn't be this much more
> > safe for the future?
>
> I didn't realise that glibc provided wrapper's for either ioprio_set()
> or ioprio_get(). If they do, then I agree that the patch should use the
> glibc calls.
I don't see any wrappers for this in glibc. (I'm looking at 2.9.)
Perhaps Jörg was being optimistic? :-)
The definition of the ioprio_set function above is identical to that in
util-linux/schedutils/ionice.c, aside from trivial whitespace
differences.
> > > @@ -1334,6 +1355,12 @@
> > > fatal("Unable to alter nice level by %i: %s",
> > > nicelevel, strerror(errno));
> > > }
> > > + if (ioprio) {
> > > + errno = 0;
> > > + if ((ioprio_set(IOPRIO_WHO_PROCESS, getpid(), ioprio << 13) ==
> > > -1) && (errno != 0))
> >
> > To me, the macro IOPRIO_PRIO_DATA would be more readable than << 13.
> > And I miss the possibility to select a different scheduler class.
>
> This patch adjusts the scheduler class already - although this probably
> isn't obvious with the naming conventions I have chosen (according to
> linux/ioprio.h, the upper 3 bits of the 16 bit ioprio are the scheduler
> class, and these are the bits that this patch allows you to configure).
> The lower 13 bits just end up being set to zero, which is probably not
> the correct thing to do anyway. I'd be more than happy to modify the
> patch to allow you to specify both IO scheduler class and IO scheduler
> class data, in the same way that the ionice utility already allows.
Although I'm sure I could work with anything sufficiently expressive, it
seems to me that making the interface parallel that of ionice would be
the course of least confusion.
--
Colin Watson [cjwat...@ubuntu.com]
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
