Re: filedesc_sx patch (20070401a)

On Wednesday 04 April 2007 00:01, Robert Watson wrote:
> On Sun, 1 Apr 2007, Robert Watson wrote:
> > The attached patch moves file descriptor locks from being a custom
> > mutex/sleep lock implemented using msleep() to an sx lock.  With the new
> > sx lock optimizations in place, this is now sensible, avoiding both a
> > custom lock type and significantly improving performance.  Kris has
> > reported 2x-4x improvement in transactions/sec with MySQL using this
> > patch, as it greatly reduces the cost of lock contention during file
> > descriptor lookup for threaded applications, and also moves to shared
> > locking to avoid exclusive acquisition for read-only operations (the vast
> > majority in most workloads). Patch is below, but you can also download
> > from:
> >
> >  http://www.watson.org/~robert/freebsd/netperf/20070401a-filedesc-sx.diff
> >
> > I'm currently waiting for the sx lock changes to settle for a few days
> > before committing, so will plan to commit this around Wednesday/Thursday
> > of this week (unless serious problems arise).
>
> Andrzej has pointed out that shortly after I posted the patch, it came into
> conflict with changes in VFS.  I've updated the patch and posted it at:
>
>    http://www.watson.org/~robert/freebsd/netperf/20070403-filedesc-sx.diff

Just a small comment:

_at__at_ -60,10 +60,7 _at__at_
 	u_short	fd_cmask;		/* mask for file creation */
 	u_short	fd_refcnt;		/* thread reference count */
 	u_short	fd_holdcnt;		/* hold count on structure + mutex */
-
-	struct	mtx fd_mtx;		/* protects members of this struct */
-	int	fd_locked;		/* long lock flag */
-	int	fd_wanted;		/* "" */
+	struct	sx fd_sx;		/* protects members of this struct */
 	struct	kqlist fd_kqlist;	/* list of kqueues on this filedesc */
 	int	fd_holdleaderscount;	/* block fdfree() for shared close() */
 	int	fd_holdleaderswakeup;	/* fdfree() needs wakeup */

Maybe it is better if you order the elements by size. Then you don't waste so 
many extra bytes on platforms where the fields must be aligned.

struct {
  struct sx fd_sx;
  struct kqlist fd_kqlist;

  int ...;

  u_short ....;
};

--HPS