Bug 56318 - fflush() on synth target does not work
fflush() on synth target does not work
Status: CLOSED WONTFIX
Product: eCos
Classification: Retired
Component: HAL (Show other bugs)
1.5.2
i386 Linux
medium Severity medium
: ---
: ---
Assigned To: Jonathan Larmour
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2001-11-15 10:49 EST by Andrew Lunn
Modified: 2007-04-18 12:38 EDT (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2003-06-20 12:06:31 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Proposed solution (6.74 KB, patch)
2002-09-06 11:25 EDT, Andrew Lunn
no flags Details | Diff
Patch version 2 (8.47 KB, patch)
2002-09-06 13:23 EDT, Andrew Lunn
no flags Details | Diff

  None (edit)
Description Andrew Lunn 2001-11-15 10:49:25 EST
Description of Problem:

The fflush() call on the synth target does not work. I've not tried other
targets. Its the sort of thing i expects been tested lots, so i guess its
specific to the synth HAL. 

How Reproducible: 100%

main(int argc, char * argv[]) {

  fputs("hello world again",stdout);
  fflush(stdout);

  exit(1);
}

Steps to Reproduce:
1. Compile program above.
2. Run
3. 

Actual Results:


Expected Results:

hello world again
Comment 1 Andrew Lunn 2001-11-16 04:52:08 EST
Yep, its a synth hal problem. hal_diag_write_char buffers up charactors until
either a \n or 128 charactors are buffered. It then make one system call to
write out the buffer and a second one to flush it. 

There does not appear to be a way for libc to cause this buffer to be flushed.
Comment 2 Nick Garnett 2001-11-20 05:57:59 EST
This may be a more generic bug in the libc interface to the HAL serial drivers.
We may need to add a HAL level serial flush macro to make this work.

An alternative would be to flush the output in the synthetic target after every
character.

I'll leave it to bart to decide what is best here. I'm not really sure why it
was sent to me.
Comment 3 Jonathan Larmour 2001-11-21 14:15:34 EST
The steps to do this would be pretty easy. libc already calls cyg_io_get_config
with the CYG_IO_GET_CONFIG_SERIAL_OUTPUT_DRAIN key (or fsync() which for "devfs"
calls cyg_io_get_config the same way). So nothing is required there in libc.

The only difficulty is passing this request into the haldiag pseudo driver in
io/serial/current/src/common/haldiag.c. Once that is solved we could just
provide a macro like HAL_DIAG_FLUSH_OUTPUT in hal_diag.h.

But I'm not sure of the best way to do that because serial drivers are only
considered to have set_config, not get_config functions. I think maybe it would
be best just to flush the output for every character in the synth target. I
agree with Nick that Bart can decide. I just wanted to clarify the structure a
bit.

Oh, and Nick, you were the lucky winner because bugzilla insists on an initial
owner, so you own the entire HAL component in bugzilla. But don't worry too much
- everyone else is CC'd.

This is only temporary - we'll be changing bugzilla to send to mailing lists
instead "soon". And hopefully eventually have different owners for different HAL
architectures.
Comment 4 Bart Veer 2001-12-07 09:39:34 EST
System calls are not that cheap, so I am not particularly keen on the idea of
doing a system call for every single character. This is especially true
if you have a system which does lots of output, e.g. a configuration with
tracing enabled and unbuffered. Some way of getting fflush() down to
the HAL would seem preferable.

I do not understand the exact route by which libc operations get down to
hal_diag_write_char(), but looking at some other code I see references to
DIAG_DEVICE_END_SYNC() and diag_device_end_sync(). Are these relevant somehow,
i.e. are there optional functions which the synthetic target HAL could
provide to give the desired behaviour?

Comment 5 Jonathan Larmour 2001-12-07 10:07:58 EST
I don't think they are relevant, no. The flush happens within libc stdio by it
calling cyg_stdio_flush in include/io.inl. That in turn either calls:

cyg_io_get_config(dev,
                             CYG_IO_GET_CONFIG_SERIAL_OUTPUT_DRAIN,
                             NULL, NULL);
or if we have CYGPKG_IO_FILEIO, fsync( dev ), which deeper down in fileio's
devfs calls the exact same cyg_io_get_config line above.

That cyg_io_get_config will be interpreted by the code in
io/serial/current/src/common/serial.c which _only_ drains the serial buffer at
that level. It does not contact any lower layer.

Maybe despite the naming oddity, we should just add a
CYG_IO_SET_CONFIG_HW_SERIAL_OUTPUT_DRAIN config key, which is passed into the
lower level driver (and the error code is ignored because most drivers won't
support it). So haldiag_set_config in io/serial/current/src/common/haldiag.c can
act on it and call a HAL_DIAG_FLUSH_OUTPUT macro if defined.

Note that this layer goes straight to haldiag - not via infra's diag.

I believe the DIAG_DEVICE_START/END_SYNC stuff was a mutual exclusion thing -
i.e. not related to this.


Comment 6 Andrew Lunn 2002-09-06 11:21:14 EDT
Soon to be attached is a patch which seems to work. I followed what Jifl suggested.

I have some questions

1) In serial.c: serial_get_config i do the call down into the next layer to
cause the flush without the out_cbuf lock. I don't think i need it, since im not
doing anything with the buffer. Im flushing the buffer at the next level down,
not this level which has already been drained. Is this correct?

2) In config_keys.h:  From the naming convention, it looks like the new key is
in a catagory of its own. Is this correct or do you want to make it part of the
0x100 serial catagory?

3) There is a hole in the current key numbers. 0x500 is not used so i've used it
for the HW keys. Was this hole deliberate? Are there some other keys somewhere i
don't know about which are also using 0x500.

4) Do you know of any pre-compiled win32 ser_filter.exe? I cannot get it to compile.

I've tested this on synth and ebsa285. My test case works now, but without
ser_filter i cannot run many of the serial tests.
Comment 7 Andrew Lunn 2002-09-06 11:25:09 EDT
Created attachment 75264 [details]
Proposed solution
Comment 8 Jonathan Larmour 2002-09-06 12:02:48 EDT
The patch looks pretty good.

To answer your questions:

1) yes that's correct. if the lower layer needs locking, then it has to handle
that itself.

2) The name appears to be in a category of its own just because of my
imprecision when I wrote the earlier comments. It should be called
CYG_IO_SET_CONFIG_SERIAL_HW_OUTPUT_DRAIN and go in the 0x100 range.

3) This is irrelevant now, but OOI 0x500 truly is free - it's not used in the RH
codebase either.

4) If you've been delivered an eCos release for windows, there should be one in
the tools/bin directory. Otherwise I can mail one to you offline. Let me know.

Some minor comments:

o Can you make HAL_DIAG_FLUSH_OUTPUT be HAL_DIAG_FLUSH_OUTPUT() for consistency?

o Dare I mention SGML documentation? ;) For both HAL_DIAG_FLUSH_OUTPUT and the
new config key.

o Some spurious whitespace added at the end of config_keys.h (nitpick factor 9
engage ;-)).

o the ifdef in io/serial/current/src/common/haldiag.c should be around the whole
CYG_IO_SET_CONFIG_HW_SERIAL_OUTPUT_DRAIN case I think. Just that the return
value in serial.c is always ignored.
Comment 9 Andrew Lunn 2002-09-06 12:37:27 EDT
Where should be new key documentation go? 

This is really the next level down from that documented in io.sgml, section
io-serial-cyg-set-config.
Comment 10 Jonathan Larmour 2002-09-06 12:52:28 EDT
io.sgml is fine - the other serial keys are in there.

Just say it's to indicate to the underlying hardware device that it should drain
its output. -EINVAL is returned if the hardware is not capable of this, e.g. if
it is unbuffered.

If you prefer, I can update it. But it's pretty easy - just cut-n-paste the
existing CYG_IO_GET_CONFIG_SERIAL_OUTPUT_DRAIN entry.


Comment 11 Andrew Lunn 2002-09-06 13:23:14 EDT
Created attachment 75284 [details]
Patch version 2
Comment 12 Andrew Lunn 2002-09-06 13:24:54 EDT
Upps. Forgot the ChangeLog Entries.....I will do that now.
Comment 13 Alex Schuilenburg 2003-06-20 12:06:31 EDT
This bug has moved to http://bugs.ecos.sourceware.org/show_bug.cgi?id=56318

Note You need to log in before you can comment on or make changes to this bug.