Feature #11806: [PATCH] IO#advise should not raise Errno::ENOSYS

• Status: Closed
• Assignee: normalperson (Eric Wong)
• Target version: -

[normalperson (Eric Wong)]

As it is just a hint the kernel is free to ignore,
IO#advise already succeeds when posix_fadvise is not
available build time at all. Following that, if posix_fadvise
was available at build time but not implemented in the running
kernel, we should also ignore it.

• io.c (do_io_advise): do not raise on ENOSYS
• test/ruby/test_io.rb (test_advise): do not skip on Errno::ENOSYS
  (test_advise_pipe): ditto

[nobu (Nobuyoshi Nakada)]

Not raising NotImplementedError?

[nobu (Nobuyoshi Nakada)]

Or rb_sys_fail should map ESYS to NotImplementedError?

[normalperson (Eric Wong)]

[e-mail] wrote:

Or rb_sys_fail should map ESYS to NotImplementedError?

I don't think that's a good idea. We are already silent on
systems without HAVE_POSIX_FADVISE at build time.

IO#advise is only a hint to me as it wraps posix_fadvise.

Our own RDoc says:

On platforms that do not support the <em>posix_fadvise(2)</em>
system call, this method is a no-op.

Current Linux kernels ignores POSIX_FADV_NOREUSE entirely;
and it is silently ignored on O_DIRECT files.

So I think it is better for us to be silent on ENOSYS, as well.

[nobu (Nobuyoshi Nakada)]

• Status changed from Open to Assigned
• Assignee set to normalperson (Eric Wong)

OK, agreed.

• Status changed from Assigned to Closed

Applied in changeset r53047.

---

IO#advise should not raise Errno::ENOSYS

As it is just a hint the kernel is free to ignore,
IO#advise already succeeds when posix_fadvise is not
available build time at all. Following that, if posix_fadvise
was available at build time but not implemented in the running
kernel, we should also ignore it.

• io.c (do_io_advise): do not raise on ENOSYS
• test/ruby/test_io.rb (test_advise): do not skip on Errno::ENOSYS
  (test_advise_pipe): ditto
  [ruby-core:72066] [Feature #11806]

[naruse (Yui NARUSE)]

I don't know fadvise well,

By this change an application lose any way of getting the result of posix_fadvise.
Is it really sufficient?
For example how about returning errno as the return value instead of returning always nil?
(it is also OK that keep returning nil until someone request to change)

Anyway could you add a description to NEWS?

[normalperson (Eric Wong)]

[e-mail] wrote:

I don't know fadvise well,

By this change an application lose any way of getting the result of posix_fadvise.
Is it really sufficient?

I think so.

For example how about returning errno as the return value instead of returning always nil?

Maybe, but on the other hand, I doubt really cares that much.
They will also need to know exactly what kernel they're running
and what it does with fadvise.

(it is also OK that keep returning nil until someone request to change)

There is no guarantee a kernel does anything with it.
Anybody checking it is probably wasting their time...

We already return nil unconditionally on systems w/o fadvise at
build time.

Anyway could you add a description to NEWS?

Done, r53149