fsync(1) patch to do DIOCGFLUSH on character devices

[Konstantin Belousov]

On Sun, Jul 24, 2016 at 03:04:34AM +0000, Julian Hsiao wrote:
> Hi,
> 
> The fsync(1) utility is a simple wrapper around fsync(2), but I didn't find
> a similar utility for calling ioctl(DIOCGFLUSH) for character devices.
It is not similar in any reasonable interpretation of the word.

> fsync(2) is a no-op on character device files, which is a little surprising
> but makes sense I suppose (would raising EINVAL be against POSIX?).
POSIX does not mention special nodes at all, so the implementation is
free to do whatever it wants/needs.

> 
> However, conceptually the goal is the same: commit writes to permanent
> storage, so here's a small patch to fsync(1) that calls ioctl(DIOCGFLUSH)
> for character device files instead.
This ioctl() does not perform what you describe, at least not on the
level of the buffer cache/page cache for the device. It is the duty of
the filesystem (or other in-kernel consumer) to ensure that cache
is flushed when needed.

The ioctl only sends BIO_FLUSH command to the driver, which does
something to the hardware. Oftent this 'something' is either nop or have
no observable implications.

That said, I do not see the proposed change as useful.

> 
> Incidentally, turns out you need to open character device files with O_RDWR
> (or maybe O_WRONLY is enough, I didn't check) for DIOCGFLUSH, but fsync(2)
> only needs O_RDONLY for plain files. This seems a little odd; does anyone
> know why?
> 
> Julian Hsiao
> 
> Index: usr.bin/fsync/fsync.1
> ===================================================================
> diff --git a/head/usr.bin/fsync/fsync.1 b/head/usr.bin/fsync/fsync.1
> --- a/head/usr.bin/fsync/fsync.1	(revision 303213)
> +++ b/head/usr.bin/fsync/fsync.1	(working copy)
> @@ -44,6 +44,8 @@
>  .Nm
>  utility uses the
>  .Xr fsync 2
> +or the
> +.Xr ioctl 2
>  function call.
>  .Sh EXIT STATUS
>  If an error occurs, the
> Index: usr.bin/fsync/fsync.c
> ===================================================================
> diff --git a/head/usr.bin/fsync/fsync.c b/head/usr.bin/fsync/fsync.c
> --- a/head/usr.bin/fsync/fsync.c	(revision 303213)
> +++ b/head/usr.bin/fsync/fsync.c	(working copy)
> @@ -35,6 +35,10 @@
>  #include <stdlib.h>
>  #include <sysexits.h>
>  #include <unistd.h>
> +#include <string.h>
> +#include <sys/stat.h>
> +#include <sys/disk.h>
> +#include <sys/ioctl.h>
>  
>  static void	usage(void);
>  
> @@ -44,6 +48,7 @@
>  	int fd;
>  	int i;
>  	int rval;
> +	struct stat sb;
>  	
>  	if (argc < 2) {
>  		usage();
> @@ -58,11 +63,35 @@
>  				rval = EX_NOINPUT;
>  			continue;
>  		}
> +		memset(&sb, 0, sizeof(sb));
> +		if (fstat(fd, &sb) == -1) {
> +			warn("fstat %s", argv[i]);
> +			rval = EX_OSERR;
> +			continue;
> +		}
>  
> -		if (fsync(fd) == -1) {
> -			warn("fsync %s", argv[i]);
> -			if (rval == EX_OK)
> +		if (S_ISCHR(sb.st_mode)) {
> +			if (close(fd) == -1) {
> +				warn("close %s", argv[i]);
>  				rval = EX_OSERR;
> +				continue;
> +			}
> +			if ((fd = open(argv[i], O_RDWR)) == -1) {
> +				warn("open %s", argv[i]);
> +				rval = EX_NOINPUT;
> +				continue;
> +			}			
> +		
> +			if (ioctl(fd, DIOCGFLUSH) == -1) {
> +				warn("ioctl %s", argv[i]);
> +				rval = EX_OSERR;
> +			}
> +		} else {
> +			if (fsync(fd) == -1) {
> +				warn("fsync %s", argv[i]);
> +				if (rval == EX_OK)
> +					rval = EX_OSERR;
> +			}
>  		}
>  		close(fd);
>  	}
> 
> _______________________________________________
> freebsd-hackers at freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-hackers
> To unsubscribe, send any mail to "freebsd-hackers-unsubscribe at freebsd.org"