On Fri, 2003-07-18 at 09:05, Duane Wessels wrote:
> The following patch is intended to make Squid be more consistent
> with the statCounter.syscalls.disk counters.
> 
> Currently they are incremented by UFS and Diskd, but not by
> AUFS and unlinkd.  I'd like to hear from a pthreads expert
> that the changes are safe for AUFS.
> 
> 
> Index: src/unlinkd.c
> ===================================================================
> RCS file: /server/cvs-server/squid/squid/src/unlinkd.c,v
> retrieving revision 1.44.2.1
> diff -u -3 -p -r1.44.2.1 unlinkd.c
> --- src/unlinkd.c     21 Jul 2002 00:30:03 -0000      1.44.2.1
> +++ src/unlinkd.c     17 Jul 2003 22:57:15 -0000
> @@ -54,6 +54,7 @@ main(int argc, char *argv[])
>      while (fgets(buf, UNLINK_BUF_LEN, stdin)) {
>       if ((t = strchr(buf, '\n')))
>           *t = '\0';
> +     statCounter.syscalls.disk.unlinks++;
>  #if USE_TRUNCATE
>       x = truncate(buf, 0);
>  #else

This won't increment the counters: the statCounter used by cachemanager
is not the statCounter available to the external unlinkd process.

> Index: src/fs/aufs/aiops.c
> ===================================================================
> RCS file: /server/cvs-server/squid/squid/src/fs/aufs/aiops.c,v
> retrieving revision 1.12.2.7
> diff -u -3 -p -r1.12.2.7 aiops.c
> --- src/fs/aufs/aiops.c       9 Jan 2003 05:04:23 -0000       1.12.2.7
> +++ src/fs/aufs/aiops.c       17 Jul 2003 22:54:46 -0000
> @@ -608,6 +608,7 @@ squidaio_open(const char *path, int ofla
>  static void
>  squidaio_do_open(squidaio_request_t * requestp)
>  {
> +    statCounter.syscalls.disk.opens++;
>      requestp->ret = open(requestp->path, requestp->oflag, requestp->mode);
>      requestp->err = errno;
>  }
> @@ -638,7 +639,9 @@ squidaio_read(int fd, char *bufp, int bu
>  static void
>  squidaio_do_read(squidaio_request_t * requestp)
>  {
> +    statCounter.syscalls.disk.seeks++;
>      lseek(requestp->fd, requestp->offset, requestp->whence);
> +    statCounter.syscalls.disk.reads++;
>      requestp->ret = read(requestp->fd, requestp->bufferp, requestp->buflen);
>      requestp->err = errno;
>  }
> @@ -670,6 +673,7 @@ squidaio_write(int fd, char *bufp, int b
>  static void
>  squidaio_do_write(squidaio_request_t * requestp)
>  {
> +    statCounter.syscalls.disk.writes++;
>      requestp->ret = write(requestp->fd, requestp->tmpbufp, requestp->buflen);
>      requestp->err = errno;
>  }
> @@ -696,6 +700,7 @@ squidaio_close(int fd, squidaio_result_t
>  static void
>  squidaio_do_close(squidaio_request_t * requestp)
>  {
> +    statCounter.syscalls.disk.closes++;
>      requestp->ret = close(requestp->fd);
>      requestp->err = errno;
>  }
> @@ -750,6 +755,7 @@ squidaio_unlink(const char *path, squida
>  static void
>  squidaio_do_unlink(squidaio_request_t * requestp)
>  {
> +    statCounter.syscalls.disk.unlinks++;
>      requestp->ret = unlink(requestp->path);
>      requestp->err = errno;
>  }
> @@ -776,6 +782,7 @@ squidaio_truncate(const char *path, off_
>  static void
>  squidaio_do_truncate(squidaio_request_t * requestp)
>  {
> +    statCounter.syscalls.disk.unlinks++;
>      requestp->ret = truncate(requestp->path, requestp->offset);
>      requestp->err = errno;
>  }

None of the above are safe. They are all potentially racey on SMP
machines. You need to use interlocked increments to safe perform such
counts. However, aufs -does- record the syscall counts: during the
scheduling operation, not during the worker threads actual call. That is
thread safe today..

> Index: src/fs/aufs/store_dir_aufs.c
> ===================================================================
> RCS file: /server/cvs-server/squid/squid/src/fs/aufs/store_dir_aufs.c,v
> retrieving revision 1.40.2.7
> diff -u -3 -p -r1.40.2.7 store_dir_aufs.c
> --- src/fs/aufs/store_dir_aufs.c      9 Jan 2003 03:38:38 -0000       1.40.2.7
> +++ src/fs/aufs/store_dir_aufs.c      17 Jul 2003 22:49:16 -0000
> @@ -416,7 +416,6 @@ storeAufsDirRebuildFromDirectory(void *d
>           debug(47, 3) ("  %s %7d files opened so far.\n",
>               rb->sd->path, rb->counts.scancount);
>       debug(47, 9) ("file_in: fd=%d %08X\n", fd, filn);
> -     statCounter.syscalls.disk.reads++;
>       if (FD_READ_METHOD(fd, hdr_buf, SM_PAGE_SIZE) < 0) {
>           debug(47, 1) ("storeAufsDirRebuildFromDirectory: read(FD %d): %s\n",
>               fd, xstrerror());

Why are you removing this one? (Is it double counted?)

> Index: src/fs/ufs/store_dir_ufs.c
> ===================================================================
> RCS file: /server/cvs-server/squid/squid/src/fs/ufs/store_dir_ufs.c,v
> retrieving revision 1.39.2.7
> diff -u -3 -p -r1.39.2.7 store_dir_ufs.c
> --- src/fs/ufs/store_dir_ufs.c        9 Jan 2003 03:38:48 -0000       1.39.2.7
> +++ src/fs/ufs/store_dir_ufs.c        17 Jul 2003 22:55:31 -0000
> @@ -415,7 +415,6 @@ storeUfsDirRebuildFromDirectory(void *da
>           debug(47, 3) ("  %s %7d files opened so far.\n",
>               rb->sd->path, rb->counts.scancount);
>       debug(47, 9) ("file_in: fd=%d %08X\n", fd, filn);
> -     statCounter.syscalls.disk.reads++;
>       if (FD_READ_METHOD(fd, hdr_buf, SM_PAGE_SIZE) < 0) {
>           debug(47, 1) ("storeUfsDirRebuildFromDirectory: read(FD %d): %s\n",
>               fd, xstrerror());

Ditto ?

Cheers,
Rob
-- 
GPG key available at: <http://members.aardvark.net.au/lifeless/keys.txt>.

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

Reply via email to