| Message ID | 20220116083328.26524-7-jgross@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | remove device specific struct file members | expand |
Juergen Gross, le dim. 16 janv. 2022 09:33:22 +0100, a ecrit: > Allocate the file type dynamically via alloc_file_type(). > > Instead of directly accessing the files[] array use get_file_from_fd(). > > Signed-off-by: Juergen Gross <jgross@suse.com> Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org> > --- > V3: > - switch to struct file * parameter for callbacks > - use __attribute__((constructor)) > --- > blkfront.c | 92 ++++++++++++++++++++++++++++++++++------------ > include/blkfront.h | 5 --- > include/lib.h | 3 +- > lib/sys.c | 24 ------------ > 4 files changed, 69 insertions(+), 55 deletions(-) > > diff --git a/blkfront.c b/blkfront.c > index e3f42bef..ed902702 100644 > --- a/blkfront.c > +++ b/blkfront.c > @@ -59,10 +59,10 @@ void blkfront_handler(evtchn_port_t port, struct pt_regs *regs, void *data) > { > #ifdef HAVE_LIBC > struct blkfront_dev *dev = data; > - int fd = dev->fd; > + struct file *file = get_file_from_fd(dev->fd); > > - if (fd != -1) > - files[fd].read = true; > + if ( file ) > + file->read = true; > #endif > wake_up(&blkfront_queue); > } > @@ -483,9 +483,13 @@ int blkfront_aio_poll(struct blkfront_dev *dev) > > moretodo: > #ifdef HAVE_LIBC > - if (dev->fd != -1) { > - files[dev->fd].read = false; > - mb(); /* Make sure to let the handler set read to 1 before we start looking at the ring */ > + { > + struct file *file = get_file_from_fd(dev->fd); > + > + if ( file ) { > + file->read = false; > + mb(); /* Make sure to let the handler set read to 1 before we start looking at the ring */ > + } > } > #endif > > @@ -554,22 +558,11 @@ moretodo: > } > > #ifdef HAVE_LIBC > -int blkfront_open(struct blkfront_dev *dev) > +static int blkfront_posix_rwop(struct file *file, uint8_t *buf, size_t count, > + bool write) > { > - /* Silently prevent multiple opens */ > - if(dev->fd != -1) { > - return dev->fd; > - } > - dev->fd = alloc_fd(FTYPE_BLK); > - printk("blk_open(%s) -> %d\n", dev->nodename, dev->fd); > - files[dev->fd].dev = dev; > - return dev->fd; > -} > - > -int blkfront_posix_rwop(int fd, uint8_t* buf, size_t count, int write) > -{ > - struct blkfront_dev* dev = files[fd].dev; > - off_t offset = files[fd].offset; > + struct blkfront_dev *dev = file->dev; > + off_t offset = file->offset; > struct blkfront_aiocb aiocb; > unsigned long long disksize = dev->info.sectors * dev->info.sector_size; > unsigned int blocksize = dev->info.sector_size; > @@ -711,14 +704,24 @@ int blkfront_posix_rwop(int fd, uint8_t* buf, size_t count, int write) > } > > free(copybuf); > - files[fd].offset += rc; > + file->offset += rc; > return rc; > > } > > -int blkfront_posix_fstat(int fd, struct stat* buf) > +static int blkfront_posix_read(struct file *file, void *buf, size_t nbytes) > { > - struct blkfront_dev* dev = files[fd].dev; > + return blkfront_posix_rwop(file, buf, nbytes, false); > +} > + > +static int blkfront_posix_write(struct file *file, const void *buf, size_t nbytes) > +{ > + return blkfront_posix_rwop(file, (void *)buf, nbytes, true); > +} > + > +static int blkfront_posix_fstat(struct file *file, struct stat *buf) > +{ > + struct blkfront_dev *dev = file->dev; > > buf->st_mode = dev->info.mode; > buf->st_uid = 0; > @@ -728,4 +731,45 @@ int blkfront_posix_fstat(int fd, struct stat* buf) > > return 0; > } > + > +static int blkfront_close_fd(struct file *file) > +{ > + shutdown_blkfront(file->dev); > + > + return 0; > +} > + > +static const struct file_ops blk_ops = { > + .name = "blk", > + .read = blkfront_posix_read, > + .write = blkfront_posix_write, > + .lseek = lseek_default, > + .close = blkfront_close_fd, > + .fstat = blkfront_posix_fstat, > + .select_rd = select_read_flag, > +}; > + > +static unsigned int ftype_blk; > + > +__attribute__((constructor)) > +static void blkfron_initialize(void) > +{ > + ftype_blk = alloc_file_type(&blk_ops); > +} > + > +int blkfront_open(struct blkfront_dev *dev) > +{ > + struct file *file; > + > + /* Silently prevent multiple opens */ > + if ( dev->fd != -1 ) > + return dev->fd; > + > + dev->fd = alloc_fd(ftype_blk); > + printk("blk_open(%s) -> %d\n", dev->nodename, dev->fd); > + file = get_file_from_fd(dev->fd); > + file->dev = dev; > + > + return dev->fd; > +} > #endif > diff --git a/include/blkfront.h b/include/blkfront.h > index 3528af9d..7f84a0a2 100644 > --- a/include/blkfront.h > +++ b/include/blkfront.h > @@ -28,17 +28,12 @@ struct blkfront_info > }; > struct blkfront_dev *init_blkfront(char *nodename, struct blkfront_info *info); > #ifdef HAVE_LIBC > -#include <sys/stat.h> > /* POSIX IO functions: > * use blkfront_open() to get a file descriptor to the block device > * Don't use the other blkfront posix functions here directly, instead use > * read(), write(), lseek() and fstat() on the file descriptor > */ > int blkfront_open(struct blkfront_dev *dev); > -int blkfront_posix_rwop(int fd, uint8_t* buf, size_t count, int write); > -#define blkfront_posix_write(fd, buf, count) blkfront_posix_rwop(fd, (uint8_t*)buf, count, 1) > -#define blkfront_posix_read(fd, buf, count) blkfront_posix_rwop(fd, (uint8_t*)buf, count, 0) > -int blkfront_posix_fstat(int fd, struct stat* buf); > #endif > void blkfront_aio(struct blkfront_aiocb *aiocbp, int write); > #define blkfront_aio_read(aiocbp) blkfront_aio(aiocbp, 0) > diff --git a/include/lib.h b/include/lib.h > index 9b7eaae7..2f472b00 100644 > --- a/include/lib.h > +++ b/include/lib.h > @@ -166,8 +166,7 @@ extern struct wait_queue_head event_queue; > #define FTYPE_FB 6 > #define FTYPE_KBD 7 > #define FTYPE_TAP 8 > -#define FTYPE_BLK 9 > -#define FTYPE_N 10 > +#define FTYPE_N 9 > #define FTYPE_SPARE 16 > > struct file { > diff --git a/lib/sys.c b/lib/sys.c > index 39f51980..90684335 100644 > --- a/lib/sys.c > +++ b/lib/sys.c > @@ -343,11 +343,6 @@ int read(int fd, void *buf, size_t nbytes) > } > return ret * sizeof(union xenfb_in_event); > } > -#endif > -#ifdef CONFIG_BLKFRONT > - case FTYPE_BLK: { > - return blkfront_posix_read(fd, buf, nbytes); > - } > #endif > default: > break; > @@ -392,10 +387,6 @@ int write(int fd, const void *buf, size_t nbytes) > case FTYPE_TAP: > netfront_xmit(files[fd].dev, (void*) buf, nbytes); > return nbytes; > -#endif > -#ifdef CONFIG_BLKFRONT > - case FTYPE_BLK: > - return blkfront_posix_write(fd, buf, nbytes); > #endif > default: > break; > @@ -455,10 +446,6 @@ off_t lseek(int fd, off_t offset, int whence) > return ops->lseek(file, offset, whence); > > switch(file->type) { > -#ifdef CONFIG_BLKFRONT > - case FTYPE_BLK: > - break; > -#endif > case FTYPE_FILE: > break; > default: > @@ -505,11 +492,6 @@ int close(int fd) > shutdown_netfront(files[fd].dev); > break; > #endif > -#ifdef CONFIG_BLKFRONT > - case FTYPE_BLK: > - shutdown_blkfront(files[fd].dev); > - break; > -#endif > #ifdef CONFIG_KBDFRONT > case FTYPE_KBD: > shutdown_kbdfront(files[fd].dev); > @@ -591,10 +573,6 @@ int fstat(int fd, struct stat *buf) > buf->st_ctime = time(NULL); > return 0; > } > -#ifdef CONFIG_BLKFRONT > - case FTYPE_BLK: > - return blkfront_posix_fstat(fd, buf); > -#endif > default: > break; > } > @@ -710,7 +688,6 @@ static const char *const file_types[] = { > [FTYPE_CONSOLE] = "console", > [FTYPE_SOCKET] = "socket", > [FTYPE_TAP] = "net", > - [FTYPE_BLK] = "blk", > [FTYPE_KBD] = "kbd", > [FTYPE_FB] = "fb", > }; > @@ -896,7 +873,6 @@ static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exce > FD_CLR(i, exceptfds); > break; > case FTYPE_TAP: > - case FTYPE_BLK: > case FTYPE_KBD: > case FTYPE_FB: > if (FD_ISSET(i, readfds)) { > -- > 2.26.2 >
On 16/01/2022 08:33, Juergen Gross wrote: > diff --git a/blkfront.c b/blkfront.c > index e3f42bef..ed902702 100644 > --- a/blkfront.c > +++ b/blkfront.c > @@ -483,9 +483,13 @@ int blkfront_aio_poll(struct blkfront_dev *dev) > > moretodo: > #ifdef HAVE_LIBC > - if (dev->fd != -1) { > - files[dev->fd].read = false; > - mb(); /* Make sure to let the handler set read to 1 before we start looking at the ring */ > + { > + struct file *file = get_file_from_fd(dev->fd); > + > + if ( file ) { Style, as this is is being indented. > + file->read = false; > + mb(); /* Make sure to let the handler set read to 1 before we start looking at the ring */ MiniOS is in desperate need of being dragged over to LKMM, and to stop using inappropriate fences. Things will go much faster when these have all been corrected to smp barriers. ~Andrew
On 18.01.22 15:41, Andrew Cooper wrote: > On 16/01/2022 08:33, Juergen Gross wrote: >> diff --git a/blkfront.c b/blkfront.c >> index e3f42bef..ed902702 100644 >> --- a/blkfront.c >> +++ b/blkfront.c >> @@ -483,9 +483,13 @@ int blkfront_aio_poll(struct blkfront_dev *dev) >> >> moretodo: >> #ifdef HAVE_LIBC >> - if (dev->fd != -1) { >> - files[dev->fd].read = false; >> - mb(); /* Make sure to let the handler set read to 1 before we start looking at the ring */ >> + { >> + struct file *file = get_file_from_fd(dev->fd); >> + >> + if ( file ) { > > Style, as this is is being indented. Oh, missed that one. :-( > >> + file->read = false; >> + mb(); /* Make sure to let the handler set read to 1 before we start looking at the ring */ > > MiniOS is in desperate need of being dragged over to LKMM, and to stop > using inappropriate fences. Things will go much faster when these have > all been corrected to smp barriers. Indeed. Juergen
diff --git a/blkfront.c b/blkfront.c index e3f42bef..ed902702 100644 --- a/blkfront.c +++ b/blkfront.c @@ -59,10 +59,10 @@ void blkfront_handler(evtchn_port_t port, struct pt_regs *regs, void *data) { #ifdef HAVE_LIBC struct blkfront_dev *dev = data; - int fd = dev->fd; + struct file *file = get_file_from_fd(dev->fd); - if (fd != -1) - files[fd].read = true; + if ( file ) + file->read = true; #endif wake_up(&blkfront_queue); } @@ -483,9 +483,13 @@ int blkfront_aio_poll(struct blkfront_dev *dev) moretodo: #ifdef HAVE_LIBC - if (dev->fd != -1) { - files[dev->fd].read = false; - mb(); /* Make sure to let the handler set read to 1 before we start looking at the ring */ + { + struct file *file = get_file_from_fd(dev->fd); + + if ( file ) { + file->read = false; + mb(); /* Make sure to let the handler set read to 1 before we start looking at the ring */ + } } #endif @@ -554,22 +558,11 @@ moretodo: } #ifdef HAVE_LIBC -int blkfront_open(struct blkfront_dev *dev) +static int blkfront_posix_rwop(struct file *file, uint8_t *buf, size_t count, + bool write) { - /* Silently prevent multiple opens */ - if(dev->fd != -1) { - return dev->fd; - } - dev->fd = alloc_fd(FTYPE_BLK); - printk("blk_open(%s) -> %d\n", dev->nodename, dev->fd); - files[dev->fd].dev = dev; - return dev->fd; -} - -int blkfront_posix_rwop(int fd, uint8_t* buf, size_t count, int write) -{ - struct blkfront_dev* dev = files[fd].dev; - off_t offset = files[fd].offset; + struct blkfront_dev *dev = file->dev; + off_t offset = file->offset; struct blkfront_aiocb aiocb; unsigned long long disksize = dev->info.sectors * dev->info.sector_size; unsigned int blocksize = dev->info.sector_size; @@ -711,14 +704,24 @@ int blkfront_posix_rwop(int fd, uint8_t* buf, size_t count, int write) } free(copybuf); - files[fd].offset += rc; + file->offset += rc; return rc; } -int blkfront_posix_fstat(int fd, struct stat* buf) +static int blkfront_posix_read(struct file *file, void *buf, size_t nbytes) { - struct blkfront_dev* dev = files[fd].dev; + return blkfront_posix_rwop(file, buf, nbytes, false); +} + +static int blkfront_posix_write(struct file *file, const void *buf, size_t nbytes) +{ + return blkfront_posix_rwop(file, (void *)buf, nbytes, true); +} + +static int blkfront_posix_fstat(struct file *file, struct stat *buf) +{ + struct blkfront_dev *dev = file->dev; buf->st_mode = dev->info.mode; buf->st_uid = 0; @@ -728,4 +731,45 @@ int blkfront_posix_fstat(int fd, struct stat* buf) return 0; } + +static int blkfront_close_fd(struct file *file) +{ + shutdown_blkfront(file->dev); + + return 0; +} + +static const struct file_ops blk_ops = { + .name = "blk", + .read = blkfront_posix_read, + .write = blkfront_posix_write, + .lseek = lseek_default, + .close = blkfront_close_fd, + .fstat = blkfront_posix_fstat, + .select_rd = select_read_flag, +}; + +static unsigned int ftype_blk; + +__attribute__((constructor)) +static void blkfron_initialize(void) +{ + ftype_blk = alloc_file_type(&blk_ops); +} + +int blkfront_open(struct blkfront_dev *dev) +{ + struct file *file; + + /* Silently prevent multiple opens */ + if ( dev->fd != -1 ) + return dev->fd; + + dev->fd = alloc_fd(ftype_blk); + printk("blk_open(%s) -> %d\n", dev->nodename, dev->fd); + file = get_file_from_fd(dev->fd); + file->dev = dev; + + return dev->fd; +} #endif diff --git a/include/blkfront.h b/include/blkfront.h index 3528af9d..7f84a0a2 100644 --- a/include/blkfront.h +++ b/include/blkfront.h @@ -28,17 +28,12 @@ struct blkfront_info }; struct blkfront_dev *init_blkfront(char *nodename, struct blkfront_info *info); #ifdef HAVE_LIBC -#include <sys/stat.h> /* POSIX IO functions: * use blkfront_open() to get a file descriptor to the block device * Don't use the other blkfront posix functions here directly, instead use * read(), write(), lseek() and fstat() on the file descriptor */ int blkfront_open(struct blkfront_dev *dev); -int blkfront_posix_rwop(int fd, uint8_t* buf, size_t count, int write); -#define blkfront_posix_write(fd, buf, count) blkfront_posix_rwop(fd, (uint8_t*)buf, count, 1) -#define blkfront_posix_read(fd, buf, count) blkfront_posix_rwop(fd, (uint8_t*)buf, count, 0) -int blkfront_posix_fstat(int fd, struct stat* buf); #endif void blkfront_aio(struct blkfront_aiocb *aiocbp, int write); #define blkfront_aio_read(aiocbp) blkfront_aio(aiocbp, 0) diff --git a/include/lib.h b/include/lib.h index 9b7eaae7..2f472b00 100644 --- a/include/lib.h +++ b/include/lib.h @@ -166,8 +166,7 @@ extern struct wait_queue_head event_queue; #define FTYPE_FB 6 #define FTYPE_KBD 7 #define FTYPE_TAP 8 -#define FTYPE_BLK 9 -#define FTYPE_N 10 +#define FTYPE_N 9 #define FTYPE_SPARE 16 struct file { diff --git a/lib/sys.c b/lib/sys.c index 39f51980..90684335 100644 --- a/lib/sys.c +++ b/lib/sys.c @@ -343,11 +343,6 @@ int read(int fd, void *buf, size_t nbytes) } return ret * sizeof(union xenfb_in_event); } -#endif -#ifdef CONFIG_BLKFRONT - case FTYPE_BLK: { - return blkfront_posix_read(fd, buf, nbytes); - } #endif default: break; @@ -392,10 +387,6 @@ int write(int fd, const void *buf, size_t nbytes) case FTYPE_TAP: netfront_xmit(files[fd].dev, (void*) buf, nbytes); return nbytes; -#endif -#ifdef CONFIG_BLKFRONT - case FTYPE_BLK: - return blkfront_posix_write(fd, buf, nbytes); #endif default: break; @@ -455,10 +446,6 @@ off_t lseek(int fd, off_t offset, int whence) return ops->lseek(file, offset, whence); switch(file->type) { -#ifdef CONFIG_BLKFRONT - case FTYPE_BLK: - break; -#endif case FTYPE_FILE: break; default: @@ -505,11 +492,6 @@ int close(int fd) shutdown_netfront(files[fd].dev); break; #endif -#ifdef CONFIG_BLKFRONT - case FTYPE_BLK: - shutdown_blkfront(files[fd].dev); - break; -#endif #ifdef CONFIG_KBDFRONT case FTYPE_KBD: shutdown_kbdfront(files[fd].dev); @@ -591,10 +573,6 @@ int fstat(int fd, struct stat *buf) buf->st_ctime = time(NULL); return 0; } -#ifdef CONFIG_BLKFRONT - case FTYPE_BLK: - return blkfront_posix_fstat(fd, buf); -#endif default: break; } @@ -710,7 +688,6 @@ static const char *const file_types[] = { [FTYPE_CONSOLE] = "console", [FTYPE_SOCKET] = "socket", [FTYPE_TAP] = "net", - [FTYPE_BLK] = "blk", [FTYPE_KBD] = "kbd", [FTYPE_FB] = "fb", }; @@ -896,7 +873,6 @@ static int select_poll(int nfds, fd_set *readfds, fd_set *writefds, fd_set *exce FD_CLR(i, exceptfds); break; case FTYPE_TAP: - case FTYPE_BLK: case FTYPE_KBD: case FTYPE_FB: if (FD_ISSET(i, readfds)) {
Allocate the file type dynamically via alloc_file_type(). Instead of directly accessing the files[] array use get_file_from_fd(). Signed-off-by: Juergen Gross <jgross@suse.com> --- V3: - switch to struct file * parameter for callbacks - use __attribute__((constructor)) --- blkfront.c | 92 ++++++++++++++++++++++++++++++++++------------ include/blkfront.h | 5 --- include/lib.h | 3 +- lib/sys.c | 24 ------------ 4 files changed, 69 insertions(+), 55 deletions(-)