Hi Jörg,

On Wed, 2009-01-07 at 18:35 +0100, Jörg Sommer wrote: 
> Hi,
> 
> Colin Watson schrieb am Wed 07. Jan, 12:18 (+0000):
> > On Sat, Sep 22, 2007 at 09:35:25AM +0200, Dominique Dumont wrote:
> > > I use start-stop-daemon to launch hellanzb which can sometimes be very
> > > io intensive (with par2 and unrar)
> > > 
> > > It would be great if I could launch hellanzb with ionice -c idle
> > > through start-stop-daemon. Could you add an option --ioniceclass or
> > > --ionicelevel ?
> > 
> > Chris Coulson <chrisccoul...@googlemail.com> sent a patch for this to
> > the corresponding Ubuntu bug report
> > (https://bugs.launchpad.net/ubuntu/+source/dpkg/+bug/306961).
> 
> > 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.

> 
> > @@ -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.

Regards
Chris

Attachment: signature.asc
Description: This is a digitally signed message part

Reply via email to